Closed Bug 1382897 Opened 2 years ago Closed 2 years ago

Intermittent dom/media/webaudio/test/test_nodeCreationDocumentGone.html | application crashed [@ mozilla::OffTheBooksMutex::AssertCurrentThreadOwns] after IsAcquired() && mOwningThread == PR_GetCurrentThread(), at BlockingResourceBase.cpp:400

Categories

(Core :: Web Audio, defect, P2, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: freesamael)

References

Details

(Keywords: crash, intermittent-failure, Whiteboard: [stockwell fixed:product])

Crash Data

Attachments

(1 file, 1 obsolete file)

I'm looking into this. In the meantime if it's necessary just back out it until we find the root cause.
39 failures since this started, not sure if that is related to retriggers.
Whiteboard: [stockwell needswork]
Rank: 29
Priority: -- → P2
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → sawang
Status: NEW → ASSIGNED
Comment on attachment 8889708 [details] [diff] [review]
v1

Hi Sotaro,

There's a call to DidComposite from HandleDPEnd [1] added in bug 1335335. Had a short discussion with Jerry and we think it misses a lock on sIndirectLayerTreesLock. Was it intentional? Does this patch look right to you?

FWIW, it seems to be a retry on ShowRemoteFrame I added [2] somehow causes this code path hit.

[1] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/gfx/layers/wr/WebRenderBridgeParent.cpp#435
[2] https://reviewboard.mozilla.org/r/146494/diff/5/#1
Flags: needinfo?(sawang)
Attachment #8889708 - Flags: feedback?(sotaro.ikeda.g)
(In reply to Samael Wang [:freesamael] from comment #6)
> Comment on attachment 8889708 [details] [diff] [review]
> Auto lock on sIndirectLayerTreesLock in DidComposite
> 
> Hi Sotaro,
> 
> There's a call to DidComposite from HandleDPEnd [1] added in bug 1335335.
> Had a short discussion with Jerry and we think it misses a lock on
> sIndirectLayerTreesLock. Was it intentional? Does this patch look right to
> you?

I do not think it is intentional. It seems just forgot to add it. It might be better to ask :mattwoodrow about it. attachment 8889708 [details] [diff] [review] looks good to me.
Attachment #8889708 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Comment on attachment 8889708 [details] [diff] [review]
v1

Matt would you take a look? I'm trying to fix the issue that WebRenderBridgeParent::HandleDPEnd calls DidComposite without holding a lock [1]

[1] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/gfx/layers/wr/WebRenderBridgeParent.cpp#435
Attachment #8889708 - Flags: review?(matt.woodrow)
Comment on attachment 8889708 [details] [diff] [review]
v1

Review of attachment 8889708 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/CompositorBridgeParent.h
@@ +135,2 @@
>    virtual void DidComposite(uint64_t aId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd) {}
> +  virtual void DidCompositeLocked(uint64_t aId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd) {}

We don't need to define this in the base class, since we only ever call it on the cross-process version.
Attachment #8889708 - Flags: review?(matt.woodrow) → review+
Attachment #8889708 - Attachment description: Auto lock on sIndirectLayerTreesLock in DidComposite → v1
Attachment #8889708 - Attachment is obsolete: true
Not seeing the same crash on try. Let's try to land this.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e292ff93034
Auto lock on sIndirectLayerTreesLock in DidComposite. r=mattwoodrow
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4e292ff93034
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Whiteboard: [stockwell needswork] → [stockwell fixed:product]
You need to log in before you can comment on or make changes to this bug.