Plugin crash [@ gfxContext::gfxContext ] when loading "My T-Mobile" login page

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: dholbert, Assigned: romaxa)

Tracking

({crash, crashreportid, regression})

Trunk
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

(crash signature, )

Attachments

(2 attachments)

STEPS TO REPRODUCE:
 1. Load https://my.t-mobile.com/Login/MyTMobileLogin.aspx
 (If you don't have a plugin crash page, reload)

ACTUAL RESULTS: I get a plugin crash notification at that page about 20-50% of the time.

I also get a bunch of messages like:
> ###!!! [Parent][AsyncChannel] Error: Channel error: cannot send/recv
and
> ###!!! [Parent][RPCChannel] Error: Channel error: cannot send/recv
spammed to my terminal.

Sample crash report: bp-69b5a32d-b512-484c-a06e-506eb2110104
From quick code inspection from the source links given in the crash report, I'm guessing it's because we end up using "mHelperSurface" as our surface, when it's actually null, and then we crash when we try to call a method on it.

In the gfxContext constructor, we crash at this line:
> 64     mCairo = cairo_create(surface->CairoSurface());
so it looks like |surface| is null.  That's probably because, up one level, we suddenly switch to using mHelperSurface without null-checking it:
> 2594 PluginInstanceChild::PaintRectToSurface(const nsIntRect& aRect,
[...]
> 2600     nsRefPtr<gfxASurface> renderSurface = aSurface;
[...]
> 2601 #ifdef MOZ_X11
[...]
> 2616         renderSurface = mHelperSurface;
> 2618 #endif
> 2619 
> 2620     if (mIsTransparent) {
> 2621        // Clear surface content for transparent rendering
> 2622        nsRefPtr<gfxContext> ctx = new gfxContext(renderSurface);

System information:
* Ubuntu 10.10 x86_64
* Mozilla/5.0 (X11; Linux x86_64; rv:2.0b9pre) Gecko/20110104 Firefox/4.0b9pre
* Flash version: libflashplayer-square-preview3.so // 10.3 d162
I'm assuming that it's |mHelperSurface| that's null rather than |aSurface|, because we call some methods on |aSurface| up one level in "PluginInstanceChild::PaintRectWithAlphaExtraction":

> 2671     whiteImage->SetDeviceOffset(deviceOffset);
> 2672     PaintRectToSurface(aRect, whiteImage, gfxRGBA(1.0, 1.0, 1.0));
(Note that whiteImage becomes |aSurface| in the PaintRectToSurface call)

So if |aSurface| were null, then we would have crashed back at line 2671.  But we crash later than that -- so |aSurface| is presumably non-null.
Tentatively flagging as blocking the bug that added the
 "renderSurface = mHelperSurface;"
line from comment 0.
Blocks: 556487
Keywords: crash
Adding 'regression' keyword, since two old builds that I tested don't exhibit this bug:
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100914 Firefox/4.0b7pre
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100917 Firefox/4.0b7pre

The latter build there is from after the landing of bug 556487 (which I flagged as causing this bug in comment 2), so I'm reverting comment 2.
No longer blocks: 556487
Was able to reproduce this (or something like it) in nightlies as far back as 2010-10-03...
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20101003 Firefox/4.0b7pre
...though it's a lot less frequent there for some reason.  I got just one plugin-crash on reloading the "My T-mobile" page in that nightly...
 bp-ce80cd89-0760-447b-aa49-ad6512110104
...and I couldn't get more beyond that. (Note that the signature & stack there don't have any symbols for things in libxul -- maybe because we don't have symbols on crash-stats for builds that far back?)
Should I  be logged in on this page? or login page is enough to reproduce this bug?

MaybeCreatePlatformHelperSurface should create mHelperSurface, and mHelperSurface can be null only if we got somehow DoAsyncSetWindow between ShowPluginFrame and PaintRectToSurface...

And that actually might happen, because we get crash on rendering white image... and between ShowPluginFrame and PaintRectToSurface rendering to blackImage did happen. (which might rotate event loop and process DoAsyncSetWindow async call)

Haven't seen this before, but it might be related to bug 615417...
Possible options are:
1) Hold mHelperSurface in PaintRectWithAlphaExtraction
2) call MaybeCreatePlatformHelperSurface before each PaintRectToSurface in PaintRectWithAlphaExtraction
3) just stop rendering PaintRectWithAlphaExtraction if mHelperSurface become null
(In reply to comment #5)
> Should I  be logged in on this page? or login page is enough to reproduce this
> bug?

No -- no need to be logged in.  Just loading the login page itself is enough. (My testing & most of my crashes so far have been me testing with fresh profile, not logged in, just loading/reloading the URL.)
FWIW, I just reproduced this on a different machine running Ubuntu 10.10, but with the stock flashplugin-installer package (version 10.1.102.65ubuntu0.10.10.1 according to synaptic / ver 10.1 r102 according to about:plugins), and with a 32-bit cpu instead of 64-bit.

So this isn't 64-bit specific, nor is it specific to the "square" preview of Flash 10.3.

crashreport for this new machine is: bp-a3538fc4-3a0f-4580-91e5-f0d722110104
Hardware: x86_64 → All
(In reply to comment #5)
> Haven't seen this before, but it might be related to bug 615417...

Just to be clear -- if by "related to" you were suggesting "regression from": this is unlikely to be a regression from that bug, because (per comment 4) I can reproduce a plugin crash at this bug's URL in nightlies at least as far back as Oct 5th, which is 2 months before bug 615417 landed.
Hmm.. for now was not able to reproduce it on Ubuntu 10.10, Flash 10.1 r102, Built from http://hg.mozilla.org/mozilla-central/rev/be8006fc9c4a
Is it possible to analyze coredump with gdb from that crash report somehow?
Ok, got it reproducible with my build, will check what is going on.
Assignee: nobody → romaxa
Following leading to crash:
DoAsyncSetWindow - helpSurf:(nil), sz[0,0]->[941,31] Drop mHelperSurface!!
MaybeCreatePlatformHelperSurface() new helpSurf:0xaf4ee760

PaintRectWithAlphaExtraction - begin
PaintRectToSurface (blackImage)render = helpSurf:0xaf4ee790
DoAsyncSetWindow helpSurf:0xaf4ee760, sz[941,31]->[941,37] Drop mHelperSurface!!
PaintRectToSurface render:0xaf4ee6a0 = helpSurf:(nil)
Attachment #501284 - Flags: feedback?(roc)
Another version of fix for this bug
Attachment #501286 - Flags: feedback?(roc)
Attachment #501286 - Flags: feedback?(roc) → review?
blocking2.0: --- → ?
Attachment #501286 - Flags: review? → review?(roc)
We should not be processing a DoAsyncSetWindow during PaintRectWithAlphaExtraction. What is the call stack for that DoAsyncSetWindow?
Bug 624809 has some data for a similar issue on Windows.
blocking2.0: ? → final+
Depends on: 624809
Attachment #501284 - Flags: feedback?(roc) → feedback-
Attachment #501286 - Flags: review?(roc) → review-
> We should not be processing a DoAsyncSetWindow during
plugin can rotate event-loop anytime even while rendering, we even have some protection for it: mPendingPluginCall.. in PluginInstanceChild.h

Why both patches rejected here?
See bug 624809. Benjamin found the problem.

The plugin should not be spinning the event loop while processing a paint event!
should we just check for mPendingPluginCall and postpone DoAsyncSetWindow for one more iteration?
IIRC I tried to set mPendingPluginCall true/false for SetWindow call... but some tests were failing ...
Was this fixed by bug 624809?
Seems to have been, yes!

Loaded ~10 "My T-Mobile" tabs in the post-bug624809-fix nightly:
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b10pre) Gecko/20110115 Firefox/4.0b10pre
and I cycled through them 4 times with no crash notifications.

I tried the previous (pre-bug624809-fix) nightly, and after cycling through & reloading all the tabs twice, most or all tabs reported plugin crashes.

Resolving as fixed by bug 624809.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
There's now a visual glitch on the first load of the "My T-Mobile" page when it's in a background tab, though, and I think it might be related.  I filed Bug 622985 on that.
copy-paste fail... I meant to say "I filed bug 626691 on that."
Crash Signature: [@ gfxContext::gfxContext ]
You need to log in before you can comment on or make changes to this bug.