This bug was filed from the Socorro interface and is report bp-2eac13dc-a8ae-41bc-b6e0-1fdd12161008. This was noted while looking at nightly crash stats.As far as I found out the crash associated multiple users. The crash started on 2016-10-08 14:10:22 with build id 20161007030207 and is still continuing. adding ni on :marcia maybe help figure out how to place this in the right component adding ni on :Munro Mengjue in case he has some ideas,since it appears he worked in that area.

Link to all crashes on trunk: http://bit.ly/2dJChSC. Moving to correct component.

Comment on attachment 8799621 [details] Bug 1308792 - protect the critical section accessing device_info_cs_; https://reviewboard.mozilla.org/r/84762/#review83548 ::: media/webrtc/trunk/webrtc/video_engine/vie_input_manager.cc:376 (Diff revision 1) > CriticalSectionScoped cs(observer_cs_.get()); > if (observer_) { > LOG_F(LS_ERROR) << "Observer already registered."; > return -1; > } > observer_ = observer; > } > > + CriticalSectionScoped cs(device_info_cs_.get()); > if (!GetDeviceInfo()) > return -1; > > if (capture_device_info_ != NULL) > capture_device_info_->RegisterVideoInputFeedBack(*this); Do we really need to have both critial sections held at once? Can't we release one, then grab the other? (I.e. must the two updates be effectively atomic?) I always am very hesitant to write code that requires two semaphores/critical-sections/etc at the same time. That said, this does appear to be safe; the critical sections are obtained in the same order.

I also see another signature that looks to be related - [@ webrtc::ViECaptureImpl::GetCaptureDevice] - will the patch address this crash as well: http://bit.ly/2dZiz3v?

I don't see how these would be related, but I'll let Munro take a deeper look

Comment on attachment 8799621 [details] Bug 1308792 - protect the critical section accessing device_info_cs_; https://reviewboard.mozilla.org/r/84762/#review83548 > Do we really need to have both critial sections held at once? Can't we release one, then grab the other? (I.e. must the two updates be effectively atomic?) I always am very hesitant to write code that requires two semaphores/critical-sections/etc at the same time. > > That said, this does appear to be safe; the critical sections are obtained in the same order. { CriticalSectionScoped cs(observer_cs_.get()); if (observer_) { LOG_F(LS_ERROR) << "Observer already registered."; return -1; } observer_ = observer; } Yes, in this patch, we release one, then grab the other. The first critical section is enclosed in braces. CriticalSectionScoped dtor will leave the critical section automatically.

(In reply to Marcia Knous [:marcia - use ni] from comment #4) > I also see another signature that looks to be related - [@ > webrtc::ViECaptureImpl::GetCaptureDevice] - will the patch address this > crash as well: http://bit.ly/2dZiz3v? This should be different issues. It may be related to bug 1308114. I have filed bug 1309456 to trace this symptom.

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bb8fca801fc8 protect the critical section accessing device_info_cs_; r=jesup

still found out the symptom

Adding a related signature to crash stats field.

The crash occurs at the second time of creating DeviceInfoDS. In bug 1300468 (https://hg.mozilla.org/mozilla-central/rev/18722be58f18), we use DeviceInfoDSSingleton to store the pointer of DeviceInfoDS object. However, we forgot to set DeviceInfoDSSingleton::GetInfo() to NULL in DeviceInfoDS dtor when the DeviceInfoDS object is destroyed by ViEInputManager. https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/video_engine/vie_input_manager.cc#52 So the crash occurs when we create DeviceInfoDS again since DeviceInfoDSSingleton::GetInfo() will return an address which has been deleted. I suspect all recent crashes is due to this bug.

Comment on attachment 8801522 [details] [diff] [review] Bug-1308792-clear-DeviceInfoDSSingleton-GetInfo.patch Review of attachment 8801522 [details] [diff] [review]: ----------------------------------------------------------------- Should the previous patch be backed out, or was it something that was required?

I think we still need the patch Without it, there is some chance to hit race condition.

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7101334e6c7d clear DeviceInfoDSSingleton::GetInfo() in DeviceInfoDS dtor to prevent illegal access crash; r=jesup,jib