Closed
Bug 622985
Opened 14 years ago
Closed 14 years ago
Plugin crash [@ gfxContext::gfxContext ] when loading "My T-Mobile" login page
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(blocking2.0 final+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: dholbert, Assigned: romaxa)
References
()
Details
(Keywords: crash, crashreportid, regression)
Crash Data
Attachments
(2 files)
(deleted),
patch
|
benjamin
:
feedback-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review-
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•14 years ago
|
||
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.
Reporter | ||
Comment 2•14 years ago
|
||
Tentatively flagging as blocking the bug that added the
"renderSurface = mHelperSurface;"
line from comment 0.
Reporter | ||
Comment 3•14 years ago
|
||
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
Keywords: crashreportid,
regression
Reporter | ||
Updated•14 years ago
|
Reporter | ||
Comment 4•14 years ago
|
||
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?)
Assignee | ||
Comment 5•14 years ago
|
||
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...
Assignee | ||
Comment 6•14 years ago
|
||
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
Reporter | ||
Comment 7•14 years ago
|
||
(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.)
Reporter | ||
Comment 8•14 years ago
|
||
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
Reporter | ||
Comment 9•14 years ago
|
||
(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.
Assignee | ||
Comment 10•14 years ago
|
||
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
Assignee | ||
Comment 11•14 years ago
|
||
Is it possible to analyze coredump with gdb from that crash report somehow?
Assignee | ||
Comment 12•14 years ago
|
||
Ok, got it reproducible with my build, will check what is going on.
Assignee: nobody → romaxa
Assignee | ||
Comment 13•14 years ago
|
||
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)
Assignee | ||
Comment 14•14 years ago
|
||
Another version of fix for this bug
Attachment #501286 -
Flags: feedback?(roc)
Assignee | ||
Updated•14 years ago
|
Attachment #501286 -
Flags: feedback?(roc) → review?
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Updated•14 years ago
|
Attachment #501286 -
Flags: review? → review?(roc)
We should not be processing a DoAsyncSetWindow during PaintRectWithAlphaExtraction. What is the call stack for that DoAsyncSetWindow?
Comment 16•14 years ago
|
||
Bug 624809 has some data for a similar issue on Windows.
blocking2.0: ? → final+
Updated•14 years ago
|
Attachment #501284 -
Flags: feedback?(roc) → feedback-
Updated•14 years ago
|
Attachment #501286 -
Flags: review?(roc) → review-
Assignee | ||
Comment 17•14 years ago
|
||
> 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!
Assignee | ||
Comment 19•14 years ago
|
||
should we just check for mPendingPluginCall and postpone DoAsyncSetWindow for one more iteration?
Assignee | ||
Comment 20•14 years ago
|
||
IIRC I tried to set mPendingPluginCall true/false for SetWindow call... but some tests were failing ...
Comment 21•14 years ago
|
||
Was this fixed by bug 624809?
Reporter | ||
Comment 22•14 years ago
|
||
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: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 23•14 years ago
|
||
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.
Reporter | ||
Comment 24•14 years ago
|
||
copy-paste fail... I meant to say "I filed bug 626691 on that."
Updated•14 years ago
|
Crash Signature: [@ gfxContext::gfxContext ]
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•