Closed Bug 1670557 Opened 1 year ago Closed 1 year ago

Crash in [@ RtlAcquireSRWLockExclusive | mozilla::ipc::MessageChannel::NotifyImpendingShutdown]

Categories

(Core :: IPC, defect)

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox81 --- unaffected
firefox82 --- unaffected
firefox83 blocking fixed
firefox84 + fixed

People

(Reporter: gsvelto, Assigned: nika)

References

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file)

Crash report: https://crash-stats.mozilla.org/report/index/9d0283e7-464b-42ff-8ffd-358600201011

Top 10 frames of crashing thread:

0 ntdll.dll RtlAcquireSRWLockExclusive 
1 xul.dll mozilla::ipc::MessageChannel::NotifyImpendingShutdown ipc/glue/MessageChannel.cpp:2656
2 xul.dll mozilla::dom::ContentParent::MaybeBeginShutDown dom/ipc/ContentParent.cpp:1573
3 xul.dll mozilla::dom::CanonicalBrowsingContext::PendingRemotenessChange::Clear docshell/base/CanonicalBrowsingContext.cpp:1150
4 xul.dll mozilla::dom::CanonicalBrowsingContext::ChangeRemoteness docshell/base/CanonicalBrowsingContext.cpp:1221
5 xul.dll mozilla::net::DocumentLoadListener::TriggerProcessSwitch netwerk/ipc/DocumentLoadListener.cpp:1718
6 xul.dll mozilla::net::DocumentLoadListener::OnStartRequest netwerk/ipc/DocumentLoadListener.cpp:2090
7 xul.dll mozilla::net::ParentChannelListener::OnStartRequest netwerk/protocol/http/ParentChannelListener.cpp:90
8 xul.dll nsDocumentOpenInfo::OnStartRequest uriloader/base/nsURILoader.cpp:166
9 xul.dll mozilla::net::ParentProcessDocumentOpenInfo::OnStartRequest netwerk/ipc/DocumentLoadListener.cpp:320

These crashes are writes to a NULL pointer, presumably the underlying lock. They seem to have started with buildid 20200911093056 and from what I can tell are only happening on nightly.

Only one of these crashes has Fission enabled, FWIW.

The only time when mMonitor can be nullptr is before the IPC connection to the remote process has been opened, when it is still in the launching phase (i.e. https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/dom/ipc/ContentParent.cpp#2428 hasn't been called yet). I think the call to NotifyImpendingShutdown should probably be guarded by a check of !IsLaunching() to make sure we're not in that phase of content process startup.

Flags: needinfo?(nika)
Crash Signature: [@ RtlAcquireSRWLockExclusive | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] → [@ RtlAcquireSRWLockExclusive | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] [@ RtlAcquireSRWLockExclusive | trunc | trunc | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] [@ RtlAcquireSRWLockExclusive | mozilla::detail::MutexImpl::loc…
Crash Signature: mozilla::detail::MutexImpl::lock | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] → mozilla::detail::MutexImpl::lock | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] [@ pthread_mutex_trylock | mozilla::detail::MutexImpl::lock | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] [@ __pthread_mutex_lock | mozilla::detail::Mu…
Assignee: nobody → nika
Duplicate of this bug: 1672404
Attachment #9183471 - Attachment description: Bug 1670557 - Check IsLaunching before calling NotifyImpendingShutdown, → Bug 1670557 - Check for open channel in NotifyImpendingShutdown,
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eb084006d41f
Check for open channel in NotifyImpendingShutdown, r=annyG
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

Nika, can we uplift this patch to beta? Thanks

Flags: needinfo?(nika)

Added a new signature that was briefly visible on macOS just before the fix landed.

Crash Signature: mozilla::detail::MutexImpl::lock | mozilla::ipc::MessageChannel::NotifyImpendingShutdown ] → mozilla::detail::MutexImpl::lock | mozilla::ipc::MessageChannel::NotifyImpendingShutdown] [@ pthread_mutex_lock | mozilla::detail::MutexImpl::lock | mozilla::ipc::MessageChannel::NotifyImpendingShutdown]

Comment on attachment 9183471 [details]
Bug 1670557 - Check for open channel in NotifyImpendingShutdown,

Beta/Release Uplift Approval Request

  • User impact if declined: Crashes
  • 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:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simple patch which should avoid the underlying issue without much risk.
  • String changes made/needed: None
Flags: needinfo?(nika)
Attachment #9183471 - Flags: approval-mozilla-beta?

It looks like bug 1673711 is another failure with a new signature which used to be hidden by this assertion failure. Given that there are still issues after this is fixed, it may not be worthwhile to uplift this to beta right now.

Given the volume on beta, I think we need to uplift at least a mitigation to reduce the volume before we ship 83.

Comment on attachment 9183471 [details]
Bug 1670557 - Check for open channel in NotifyImpendingShutdown,

We are taking part 1 of bug 1673711 to mitigate the issue instead.

Attachment #9183471 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

the uplift taken for this topcrasher in b9 doesn't seem to have had the intended success unfortunately.

Flags: needinfo?(nika)

Re-opening as this clearly hasn't been fixed yet based on comment 14.

Status: RESOLVED → REOPENED
Flags: needinfo?(nika)
Resolution: FIXED → ---

I've done more analysis and I think this will be fixed by also uplifting part 2 of bug 1673711. I've made an uplift request in bug 1673711 comment 17 with more details.

Status: REOPENED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED

No crashes with the RC build so far. Calling 83 fixed based on comment 16.

See Also: → 1677534
You need to log in before you can comment on or make changes to this bug.