Closed Bug 1224833 Opened 8 years ago Closed 7 years ago

Intermittent test_getUserMedia_basicScreenshare.html | application crashed [@ mozilla::layers::ImageClientSingle::UpdateImage(mozilla::layers::ImageContainer*, unsigned int)] after Assertion failure: texture == mBuffers[i].mTextureClient

Categories

(Core :: Graphics: Layers, defect)

Product:
Core
Component:
Graphics: Layers
Version:
45 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla47
Tracking Flags:
Tracking Status
firefox45 --- wontfix
firefox46 --- fixed
firefox47 --- fixed
firefox-esr45 --- fixed

People

(Reporter: philor, Assigned: sotaro)

Details

(Keywords: assertion, intermittent-failure, Whiteboard: gfx-noted)

Attachments

(2 files, 1 obsolete file)

crash.txt
7.56 KB, text/plain
Details
patch - Check explicitly if image has TextureClient
1.07 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
Whiteboard: gfx-noted
Attached file crash.txt — 
This one reproduces in fresh Nightly for me with https://jsfiddle.net/Lfru4g4n/

I hit all the different resolution buttons for a while until it happens (~5-10 clicks). Debug output attached.
Hit this assertion on test_peerConnection_removeVideoTrack.html too.

https://treeherder.mozilla.org/logviewer.html#?job_id=14181663&repo=try

I can't change this bug's summery because it's too long.
Assignee: nobody → sotaro.ikeda.g
The cause of the problem seems to become clear. In ImageClientSingle::UpdateImage(), if the function tries to forward multiple RecyclingPlanarYCbCrImages, the situation could happen. RecyclingPlanarYCbCrImages does not have TextureClient, multiple TextureClients could be created for one RecyclingPlanarYCbCrImages. Then the TextureClients are stored in ImageClientSingle::mBuffers.
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> The cause of the problem seems to become clear. In
> ImageClientSingle::UpdateImage(), if the function tries to forward multiple
> RecyclingPlanarYCbCrImages, the situation could happen.
> RecyclingPlanarYCbCrImages does not have TextureClient, multiple
> TextureClients could be created for one RecyclingPlanarYCbCrImages. Then the
> TextureClients are stored in ImageClientSingle::mBuffers.

The above was confirmed based on the link of comment 1.
Attachment #8715277 - Flags: review?(nical.bugzilla)
Comment on attachment 8715277 [details] [diff] [review]
patch - Use image->GetTextureClient(this)

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

::: gfx/layers/client/ImageClient.cpp
@@ +181,5 @@
>  
>      for (int32_t i = mBuffers.Length() - 1; i >= 0; --i) {
>        if (mBuffers[i].mImageSerial == image->GetSerial()) {
> +        if (image->GetTextureClient(this)) {
> +          MOZ_ASSERT(image->GetTextureClient(this) == mBuffers[i].mTextureClient);

I would much prefer that you create a constant boolean outside of the loop that is true if texture is non-null, and check that boolean in the loop rather than calling into GetTextureClient each time. At least we'll quickly see that checking the boolean doesn't have side effects (to make sure of that with GetTextureClient, we have to read every implementation of the method and hope that no side effects will appear in the future is an implementation is added or modified).
Attachment #8715277 - Flags: review?(nical.bugzilla) → review+
Apply the comment. Carry "r=nical".
Attachment #8715277 - Attachment is obsolete: true
Attachment #8715615 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/b0c2f52f52f1
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Changes a debug-only check, so uplifting this a=NPOTB.

https://hg.mozilla.org/releases/mozilla-aurora/rev/b36320f44174
status-firefox45: affected → wontfix
status-firefox46: --- → fixed
status-firefox-esr45: --- → affected
Whiteboard: gfx-noted → gfx-noted [checkin-needed-esr45]
You need to log in before you can comment on or make changes to this bug.