1130607 - Sporadic assertion when loading m.mogujie.com: "Assertion failure: mIsMultiPartChannel || !mImage (Already have an image for non-multipart request), at imgRequest.cpp:652"

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Daniel Holbert [:dholbert] (PTO until 2/24)]

I just hit this assertion, after loading http://m.mogujie.com/ :
{
Assertion failure: mIsMultiPartChannel || !mImage (Already have an image for non-multipart request), at image/src/imgRequest.cpp:652
}

I was running a debug build, in Responsive Design Mode (320x480, the default size), with "User Agent Overrider" addon, spoofing a Firefox for Android user agent. I tried again with a different profile (but that same addon & user agent), and I didn't hit the assertion that time. So, this must be a bit random / sporadic.

My build is based off of mozilla-central changeset e0f56d3f1fc0.

I'll post a text file with info from gdb.

Comment 1

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Attachment: information captured from gdb

We're failing the assertion because both "or" conditions are false. (mIsMultiPartChannel is false, and mImage is non-null.)

Here's information captured from gdb when I hit the fatal assertion.

Comment 2

[Daniel Holbert [:dholbert] (PTO until 2/24)]

I just hit this again (on a different machine), FWIW, so this wasn't just a random one-off occurrence.

In case it matters, I've had my patch for bug 1107378 applied in this debug build, both times that I hit this. (I'm not sure that patch is required to trigger the bug, though; I'd be a bit surprised if it were.)

Comment 3

[Benoit Girard (:BenWa)]

:seth does this need attention?

Comment 4

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Neither seth nor I can reproduce right now, (un?)fortunately. :-/

I've tested http://m.mogujie.com/ a bunch, with and without my patch queue for bug 1107378 applied on top of current mozilla-central (and with the unprefixing pref introduced in that patch-queue flipped on & off), with no assertions yet today.

Comment 5

[Seth Fowler [:seth] [:s2h]]

(In reply to Benoit Girard (:BenWa) from comment #3)
> :seth does this need attention?

Neither I nor Daniel can reproduce this again. (I've gone to great lengths to try.)

However, based upon the information Daniel collected from GDB, it looks a lot like we're getting OnStartRequest delivered more than once for a non-multipart request. This is likely a bug in Necko (or perhaps in imgLoader, although it's not obvious to me how it could happen there), and has such a great potential to cause havoc that I do think we should take some sort of action.

What I'm going to do is make us cancel the request in imgRequest::OnStartRequest in the situation where we'd hit this assertion. (I'll still keep the assertion, so hopefully we'll eventually be able to track this down.) That will minimize the damage that this issue can cause.

Comment 6

[Seth Fowler [:seth] [:s2h]]

Attachment: Cancel our imgRequest if we get an unexpected duplicate imgRequest event

Here's the patch. As I mentioned in comment 5, it doesn't fix the underlying
issue, but it should prevent any negative consequences for endusers until we can
figure it out.

Comment 7

[Seth Fowler [:seth] [:s2h]]

Thanks for the review! Try job here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=239a4be4e2e0

Comment 8

[Seth Fowler [:seth] [:s2h]]

Try looks good! Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bcb6c9232a1

Comment 9

[Carsten Book [:Tomcat]]

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=25f00181e134 since one of this changes caused a perma failure like:

https://treeherder.mozilla.org/logviewer.html#?job_id=7169706&repo=mozilla-inbound

Comment 10

[Seth Fowler [:seth] [:s2h]]

Pretty sure that's unrelated to this patch, but I pushed a new B2G-only try job just to be sure:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca3aa6a5746d

Comment 11

[Seth Fowler [:seth] [:s2h]]

This patch was unrelated. Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/be76c67b791a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be76c67b791a

Comment 13

[Seth Fowler [:seth] [:s2h]]

Patrick, thanks for pointing out bug 1166133! It looks like we have confirmation that multiple deliveries of OnStartRequest were possible, and the underlying issue seems to have been fixed in that bug.

I'm going to go ahead and resolve this on the assumption that bug 1166133 was the real fix.

Comment 14

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.