1518612 - Crash in _PR_NativeRunThread | pr_root

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

This bug was filed from the Socorro interface and is
report bp-21efc808-5c58-499a-857b-9a3f60190107.

=============================================================

Not a new crash but some reports may be security-sensitive.
This signature started to spike in 65.0b7, with 39 crashes in 37 installs. 

Top 8 frames of crashing thread:

0  @0xaec00835 
1 nss3.dll static void _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:397
2 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:137
3 ucrtbase.dll _o____lc_collate_cp_func 
4 kernel32.dll BaseThreadInitThunk 
5 mozglue.dll static void patched_BaseThreadInitThunk mozglue/build/WindowsDllBlocklist.cpp:722
6 ntdll.dll __RtlUserThreadStart 
7 ntdll.dll _RtlUserThreadStart 

=============================================================

Comment 1

[Daniel Veditz [:dveditz]]

not a very useful stack. Any correlations with 3rd party modules that might be interfering? Pretty sharp rise from nothing to many daily on Aug 15.

Did we make any Windows DLL blocklist changes around then?

Comment 2

[Carl Corcoran [:ccorcoran]]

The raw minidump reveals a better stack at the crash site:

00 2611fb6c 578fac6c 0xaec00835
01 2611fb78 6aeeebdb xul!mozilla::widget::SpawnASCReleaseThread::<unnamed-tag>::__invoke+0xc [z:\build\build\src\widget\windows\AudioSession.cpp @ 262] 
02 2611fba0 6aee07ad nss3!_PR_NativeRunThread+0x11b [z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c @ 397] 
03 2611fbac 71bae16f nss3!pr_root+0xd [z:\build\build\src\nsprpub\pr\src\md\windows\w95thred.c @ 137] 
04 2611fbe8 754c343d ucrtbase!thread_start<unsigned int (__stdcall*)(void *)>+0x3f

That thread routine is

      [](void* aRawPtr) {
        static_cast<IAudioSessionControl*>(aRawPtr)->Release();
      },

So aRawPtr is invalid here. It's passed in from here, with a helpful comment.

void AudioSession::StopInternal() {
  ...
  // Win7 is the only Windows version supported before Win8.
  if (mAudioSessionControl && !IsWin8OrLater()) {
    // bug 1419488: Avoid hanging due to Win7 race condition when destroying
    // AudioSessionControl.  We do that by Moving the AudioSessionControl
    // to a worker thread (that we never 'join') for destruction.
    SpawnASCReleaseThread(std::move(mAudioSessionControl));
  }
  ...
}

I suspect then this is something that should be investigated further in conjunction with Bug 1419488.

Comment 3

[Carl Corcoran [:ccorcoran]]

Other reports with this signature are unrelated to the above minidump analysis. For example report 02e70299-bb50-495a-ac86-8ff6b0190110 has the following stack trace:

00 xul!mozilla::`anonymous namespace'::RunWatchdog+0x8a [z:\build\build\src\toolkit\components\terminator\nsTerminator.cpp @ 200] 
01 nss3!_PR_NativeRunThread+0x11b [z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c @ 397] 
02 nss3!pr_root+0xd [z:\build\build\src\nsprpub\pr\src\md\windows\w95thred.c @ 137] 
03 ucrtbase!thread_start<unsigned int (__stdcall*)(void *)>+0x3f
04 kernel32!BaseThreadInitThunk+0xe
05 mozglue!patched_BaseThreadInitThunk+0x64 [z:\build\build\src\mozglue\build\WindowsDllBlocklist.cpp @ 715] 
06 ntdll!__RtlUserThreadStart+0x70
07 ntdll!_RtlUserThreadStart+0x1b

This suggests the crash signature is not being generated accurately for these reports.

Comment 4

[Carl Corcoran [:ccorcoran]]

NI willkg: Any idea why Socorro is processing the stack in this way? The downloaded .pdb symbols seem OK, but Socorro isn't symbolicating the bottom stack frame under _PR_NativeRunThread, resulting in an unhelpful stack signature.

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

I don't know enough about stacks and symbols.

Ted: Any ideas?

Comment 6

[Liz Henry (:lizzard)]

Marco, can you help here?

Comment 7

[Marco Castelluccio [:marco]]

No idea why we're failing to symbolicate, but maybe Gabriele can help.

Comment 8

[Jim Mathies [:jimm]]

Audio session related code, maybe David has some ideas on the cause.

Comment 9

[Gabriele Svelto [:gsvelto]]

The top frame is not being symbolicated because the instruction pointer is typically outside of xul.dll. More importantly the crash address is often the instruction pointer itself. My best guess is that it jumped through a function pointer which was clobbered with data before it was encountered. More interestingly the crash address is 0xbaadbaad in some occasions like this report:

https://crash-stats.mozilla.com/report/index/d1849860-1e6c-47d6-9db8-0f9200190120

(ignore the fact that it starts with 0xffffffff, it's a bug in Breakpad, the real address is 0xbaadbaad)

This is a suffix that cubeb appends after initializing a stream:

https://searchfox.org/mozilla-central/rev/330daedbeac2bba296d663668e0e0cf248bc6823/media/libcubeb/src/cubeb_audiotrack.c#320

This suggest this might be an off-by-one error (actually off-by-4-bytes) where the structure which contains the pointer overlaps the end of the stream.

Comment 10

[(not currently active) Ted Mielczarek]

(In reply to Carl Corcoran [:ccorcoran] from comment #2)

The raw minidump reveals a better stack at the crash site:

The first frame going missing is a long-standing known issue: bug 1310314.

(In reply to Carl Corcoran [:ccorcoran] from comment #3)

Other reports with this signature are unrelated to the above minidump analysis. For example report 02e70299-bb50-495a-ac86-8ff6b0190110 has the following stack trace:

FWIW, that report shows missing symbols for xul.dll (hence the frames highlighted in red). The root cause there is that we're missing the debug identifier / debug filename for xul.dll (if you look at the modules list you can see that they're missing). We've seen this before for OOM or near-OOM crashes. We have some code that reserves memory and tries to free it before writing a minidump to work around this problem, but it may not be 100% reliable.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Since the crash volume is low (less than 15 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit BugBot documentation.

Comment 13

[Ray Kraesig [:rkraesig]]

The bug we were concerned about four or five years ago appears to have vanished as of bug 1617283 (Fx 100) which removed the function :ccocoran identified in comment 2, and more generally. fixed our COM handling for the audio session. The vast majority of crashes over the last six months have been from Firefox versions preceding this (mostly ESRs). (As an irrelevant side note: the most common crash address seems to have been 0x20656220, which is the ASCII string "be", specifically taken from the string "This program cannot be run in DOS mode." near the start of AudioSes.dll.) Closing as RESOLVED FIXED.

---

There are nonetheless a number of crashes from more recent Firefox versions that have been bucketed into this signature, and which might therefore have been cause to open another bug.

• The most concerning, at a surface level, are the crashes whose reason is given as EXCEPTION_STACK_BUFFER_OVERRUN / FAST_FAIL_STACK_COOKIE_CHECK_FAILURE. On examination these turn out not to have been stack buffer overruns, but unrelated single-bit errors in the stored stack-cookie, presumably due to bad memory.

• Several EXCEPTION_BREAKPOINT errors are actually this shutdown hang, apparently misbucketed due to incorrect module version-info being stored in the minidump. (That issue is being discussed on #crashreporting, and may yet lead to another bug being filed.)

• All other post-v100 minidumps examined appear to have a corrupt stack (and so were merely been assigned this signature as the first recognizable frame), or have some other memory-corruption error (and don't represent a bug in Firefox). The volume is low enough that hardware issues would plausibly account for all of them.