Crash in [@ nsCOMPtr<T>::operator= | mozilla::ThreadEventTarget::Dispatch]
Categories
(Core :: WebRTC: Audio/Video, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr102 | --- | unaffected |
firefox111 | --- | disabled |
firefox112 | --- | disabled |
firefox113 | --- | fixed |
firefox114 | --- | fixed |
People
(Reporter: pehrsons, Assigned: pehrsons)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details | Review |
Consider this a followup to bug 1817724 to evaluate the results. There were no fixes to the crashes in bug 1817724, not even hypothetical. But the frame callback will behave differently so I'm hoping it will result in somewhat different symptoms.
+++ This bug was initially created as a clone of Bug #1817724 +++
+++ This bug was initially created as a clone of Bug #1811641 +++
Crash report: https://crash-stats.mozilla.org/report/index/8ad89e28-c397-49ab-bff6-616b70230119
Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Top 10 frames of crashing thread:
0 XUL nsCOMPtr<nsIThreadObserver>::operator= xpcom/base/nsCOMPtr.h:663
0 XUL mozilla::ThreadEventQueue::PutEventInternal xpcom/threads/ThreadEventQueue.cpp:140
0 XUL mozilla::ThreadEventQueue::PutEvent xpcom/threads/ThreadEventQueue.cpp:73
0 XUL mozilla::ThreadEventTarget::Dispatch xpcom/threads/ThreadEventTarget.cpp:122
0 XUL nsThread::Dispatch xpcom/threads/nsThread.cpp:676
1 XUL nsIEventTarget::Dispatch dist/include/nsIEventTarget.h:42
1 XUL mozilla::camera::CamerasParent::RecvStartCapture const dom/media/systemservices/CamerasParent.cpp:970
1 XUL mozilla::media::LambdaRunnable<mozilla::camera::CamerasParent::RecvStartCapture dom/media/systemservices/MediaUtils.h:77
2 XUL nsThread::ProcessNextEvent xpcom/ds/nsTObserverArray.h:89
3 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:473
This signature is pretty bad, but I opened a half dozen of these crashes and they all had CamerasParent::RecvStartCapture on the stack and are all on Mac OS, so I'm going to guess that this is fallout from bug 1806605.
Assignee | ||
Updated•2 years ago
|
Comment 1•2 years ago
|
||
Set release status flags based on info from the regressing bug 1806605
Assignee | ||
Comment 2•2 years ago
|
||
I found an issue based on this crash report and its stack:
0 libsystem_pthread.dylib _pthread_cond_updateval
1 libsystem_pthread.dylib pthread_cond_signal
2 libmozglue.dylib mozilla::detail::ConditionVariableImpl::notify_one() mozglue/misc/ConditionVariable_posix.cpp:93
3 XUL __59-[RTCCameraVideoCapturer stopCaptureWithCompletionHandler:]_block_invoke third_party/libwebrtc/sdk/objc/components/capturer/RTCCameraVideoCapturer.m:220
This indicates something has gone south with the monitor in the completionHandler
block we pass to stopCaptureWithCompletionHandler
, i.e. here.
The obvious issue is that we set done = true
without holding the lock which lets the calling thread stop waiting for the Notify() and run to completion, basically destroying the monitor before or while the completion handler is in Monitor::Notify(). Who knows where it could drive off to then.
Assignee | ||
Comment 3•2 years ago
|
||
Assignee | ||
Comment 5•2 years ago
|
||
Comment on attachment 9328850 [details]
Bug 1825514 - In VideoCaptureAvFoundation, lock the monitor before notifying it. r?webrtc-reviewers
Beta/Release Uplift Approval Request
- User impact if declined: This is a hypthetical fix (to a real potential issue), but if this is indeed the proper fix for the crashes tracked in this bug, uplifting now will let us save one release cycle before knowing (and before shipping), since this backend is only enabled in nightly and early beta (and only beta really gives us enough crash reports to know for sure).
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- 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): Trivially fixing a classic bug of not locking a monitor before Notify().
- String changes made/needed:
- Is Android affected?: No
Comment 6•2 years ago
|
||
bugherder |
Comment 7•2 years ago
|
||
Comment on attachment 9328850 [details]
Bug 1825514 - In VideoCaptureAvFoundation, lock the monitor before notifying it. r?webrtc-reviewers
Approved for 113.0b5. We should be able to see what happens to the crash rate for b5 & b6 before the early beta period ends this cycle.
Comment 8•2 years ago
|
||
bugherder uplift |
Description
•