Closed Bug 1614585 Opened 4 years ago Closed 4 years ago

Crash in [@ IPCError-browser | ShutDownKill | RtlpWalkFrameChain | RtlWalkFrameChain | RtlCaptureStackBackTrace | CDeviceEnumerator::UnregisterEndpointNotificationCallback]

Categories

(Core :: Widget: Win32, defect, P1)

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 --- wontfix
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- fixed
firefox76 --- fixed

People

(Reporter: achronop, Assigned: handyman)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-fe9a5f1a-bf7a-4be1-8253-379ca0200210.

Top 10 frames of crashing thread:

0 ntdll.dll RtlpWalkFrameChain 
1 ntdll.dll RtlWalkFrameChain 
2 ntdll.dll RtlCaptureStackBackTrace 
3 mmdevapi.dll CDeviceEnumerator::UnregisterEndpointNotificationCallback 
4 audioses.dll CAudioSessionControl::FinalRelease 
5 audioses.dll ATL::CComObject<CAudioSessionControl>::~CComObject<CAudioSessionControl> 
6 audioses.dll ATL::CComObject<CAudioSessionControl>::`vector deleting destructor' 
7 audioses.dll ATL::CComObject<CAudioSessionControl>::Release 
8 xul.dll mozilla::widget::AudioSession::StopInternal widget/windows/AudioSession.cpp
9 xul.dll mozilla::widget::AudioSession::Stop widget/windows/AudioSession.cpp:303

This bumped after buildid: 20200116214549. It was low traffic till then.

There's audio frames on the stack, this looks like a duplicate of bug 1614321.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE

Discussion in bug 1614321 comment 4 suggests that this may not be a dupe of that bug (not entirely, anyway). I believe we can fix this bug by extending this code [1] to work with other Windows versions.

[1] https://searchfox.org/mozilla-central/rev/c1e3d3edd4a9b784971555dc74a5de23d768b2e1/widget/windows/AudioSession.cpp#281

Assignee: nobody → davidp99
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jmathies)
Priority: -- → P1
Flags: needinfo?(jmathies)

Bug 1419488 moved AudioSession shutdown to a background thread on Windows 7 because it was leading to shutdown timeouts there. Since then, audio seems to be inspiring timeouts on other versions of Windows as well. This patch extends the Windows 7 work to all versions of Windows.

Bug 1430907 is removing AudioSession from content processes. This is the only place we have seen these crashes but AudioSession is also used in the main and plugin processes, so we want this patch to preempt issues with those processes.

Crash Signature: [@ IPCError-browser | ShutDownKill | RtlpWalkFrameChain | RtlWalkFrameChain | RtlCaptureStackBackTrace | CDeviceEnumerator::UnregisterEndpointNotificationCallback] → [@ IPCError-browser | ShutDownKill | RtlpWalkFrameChain | RtlWalkFrameChain | RtlCaptureStackBackTrace | CDeviceEnumerator::UnregisterEndpointNotificationCallback] [@ IPCError-browser | ShutDownKill | RtlpLookupFunctionEntryForStackWalks | RtlpWalkFrameC…

I r+'d this David, if you want to land it.

Pushed by daparks@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/35c8c269b95b
Use background thread to shut down AudioSession on all Windows versions r=jmathies
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

No more crashes in the last two nightly builds and none in bug 1614321 as well - which I suspected was a duplicate of this one.

The patch landed in nightly and beta is affected.
:handyman, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(davidp99)

(In reply to Gabriele Svelto [:gsvelto] from comment #8)

No more crashes in the last two nightly builds and none in bug 1614321 as well - which I suspected was a duplicate of this one.

Bug 1614321 is surprising given the unrelated stacks but I'm not complaining. I think this is good for uplift.

Flags: needinfo?(davidp99)

Comment on attachment 9129306 [details]
Bug 1614585: Use background thread to shut down AudioSession on all Windows versions r=jmathies

Beta/Release Uplift Approval Request

  • User impact if declined: Occasional crashes when content process shuts down.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: It's a concurrent timing issue -- there's no reliable repro. (I've actually personally never seen it.)
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This behavior has been in tree for Windows 7 for two years (bug 1419488). This patch just extends it to other Windows versions.
  • String changes made/needed: N/A
Attachment #9129306 - Flags: approval-mozilla-beta?

(In reply to David Parks (dparks) [:handyman] from comment #10)

Bug 1614321 is surprising given the unrelated stacks but I'm not complaining. I think this is good for uplift.

The volume hasn't dropped to zero there, but it dropped from 20-25 crashes per day to 1 or 2 per day. The remaining reports have a bunch of different and unrelated stacks, there's quite a few things mixed in there but your patch here solved all the audio-related ones.

Comment on attachment 9129306 [details]
Bug 1614585: Use background thread to shut down AudioSession on all Windows versions r=jmathies

fix a shutdown issue, approved for 75.0b5

Attachment #9129306 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: