1408693 - Crash in std::_Tree<T>::find

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Asif Youssuff]

This bug was filed from the Socorro interface and is 
report bp-aa04e1e9-4617-4582-991c-a32fa0171013.
=============================================================

Only 17 crashes in the last week, but didn't see it logged in Bugzilla. Affects v57.

Comment 1

[Jean-Yves Avenard [:jya]]

https://hg.mozilla.org/releases/mozilla-beta/annotate/b92b69f3503e/dom/media/MediaFormatReader.cpp#l109

This code isn’t thread safe. It could be accessed from multiple threads, erasing an element on one thread, to find in another is bound to create nastiness.

Comment 2

[Blake Wu [:bwu][:blakewu]]

Kaku is on "P"TO. 
Benjamin,
Can you help check it?

Comment 3

[Benjamin Chen [:bechen]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #2)
> Kaku is on "P"TO. 
> Benjamin,
> Can you help check it?

Looks like we need a mutex lock to protect it.

Comment 4

[Benjamin Chen [:bechen]]

Attachment: Bug 1408693 - Add a lock to protect the sGPUCrashDataMap.

Review commit: https://reviewboard.mozilla.org/r/189578/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189578/

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8918747 [details]
Bug 1408693 - Add a lock to protect the sGPUCrashDataMap.

https://reviewboard.mozilla.org/r/189578/#review194718

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/615798b405f4
Add a lock to protect the sGPUCrashDataMap. r=jwwang

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/615798b405f4

Comment 8

[Julien Cristau [:jcristau]]

Is this worth uplifting to 57?  Crash volume seems low, OTOH the bug sounds possibly nasty.

Comment 9

[Benjamin Chen [:bechen]]

Attachment: bug1408693.beta.patch

I believe we need uplift this patch to 57, because the telemetry result is very important for "de-blacklist". Once the crash happened, we can not gather the GPU crash status due to "de-blacklist". That will mislead us to decide the blacklist should be exist or not.

Approval Request Comment
[Feature/Bug causing the regression]: 1393399
[User impact if declined]: Low possibility crash when trying to report telemetry.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No, very hard to reproduce.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: code logic very simple.
[String changes made/needed]: none

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8919549 [details] [diff] [review]
bug1408693.beta.patch

Better telemetry data, low risk, Beta57+

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Jean-Yves, fyi, I took this patch in beta57. If you believe this one is better off staying in 58 train, please let me know. Thanks!

Comment 12

[Jean-Yves Avenard [:jya]]

I think this one is pretty safe for uplifting...

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/28f4ccb3c041