Closed Bug 300756 Opened 19 years ago Closed 19 years ago

Flash 8 Player (Public Beta) crashes Deer Park [@ js_SetupLocks]

Categories

(Core Graveyard :: Plug-ins, defect, P1)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.8beta4

People

(Reporter: aronnax07, Assigned: jst)

References

()

Details

(Keywords: crash, verified1.8, Whiteboard: [ETA 8/22][has approval])

Crash Data

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3) Gecko/20050713 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3) Gecko/20050713 Firefox/1.0+

Flash 8 crashes Deer Park very time the flash player dispatched data

Reproducible: Always

Steps to Reproduce:
1. install the Flash Player 8 Public Beta from:
http://www.macromedia.com/software/flashplayer/public_beta/
2. restart and go back to:
http://www.macromedia.com/software/flashplayer/public_beta/
3. click on (the blue text in the grey flash box): If you are having difficulty
with installation, please click here for more information
Actual Results:  
crash

Expected Results:  
open a new page
It crashes also on WinXP: TB7475272W
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050714
Firefox/1.0+ ID:2005071403
Please provide talkback ID's for the crashes.
works now, with a present day nightly - thanks ;-)
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Reopening, there is no evidence this was fixed, to my knowledge no relevant
changes were made. Resolving WFM based on comment 3. Please reopen if you find
the issue reoccurs.
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → WORKSFORME
(In reply to comment #4)
> Reopening, there is no evidence this was fixed, to my knowledge no relevant
> changes were made. Resolving WFM based on comment 3. Please reopen if you find
> the issue reoccurs.

sorry, works on the first try with a new nightly - on a second try he browser
craches again ;-(
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
reopened as per comment #5
Does it also happen if you disable bfcache ?
The winxp stack from comment #1:
js_SetupLocks 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jslock.c, line 811]
JS_SetPrivate 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 2158]
nsWindowConstructor 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/build/nsWinWidgetFactory.cpp,
line 65]
PresShell::ProcessReflowCommands 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6800]
nsBoxObject::QueryInterface 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxObject.cpp,
line 75]
PresShell::PostReflowEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6734]
DocumentViewerImpl::SetDOMDocument 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 1556]
DocumentViewerImpl::SetEnableRendering 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 1901]
Conv_06_FE_WithReverse 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsBidiUtils.cpp,
line 489]
PresShell::HandlePostedReflowCallbacks 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 5241]
PresShell::ContentStatesChanged 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 5421]
PL_HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpcom/threads/plevent.c,
line 686]
SHELL32.dll + 0x520c24 (0x778b0c24)
*** Bug 300776 has been marked as a duplicate of this bug. ***
bz: Do you have an idea where this shoould go to ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8b4?
OS: MacOS X → All
Hardware: Macintosh → All
The stack in the duped bug (bug 300776) matches the one in bug 300572 the stack
below is different. 

This may be a dupe of bug 300572 , bug 300776 is a dupe of bug 300572
I have no clue; the stack in comment 7 make no sense -- those functions don't
call each other.
Depends on: 300572
(In reply to comment #7)
> Does it also happen if you disable bfcache ?

I get a crash every time with bfcache to 3 (default);
I get no crashes so far with bfcache to 0.

TB7498968H  TB7499076K  Sorry no links; the TB-server is too busy..
Summary: Flash 8 Player (Public Beta) crashes Deer Park (Mac) → Flash 8 Player (Public Beta) crashes Deer Park
Hm, this gets weird; I only need to click the button "Home" (or any other grey
button on top of this page) to get a crash:
http://www.macromedia.com/bin/fp8betafeedback.cgi?
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB7530381Y
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=js_setuplocks&vendor=All&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

11 crashes found where the Stack Signature contains 'js_setuplocks' and the sort
is by ID.
Component: General → Plug-ins
Keywords: crash
Product: Firefox → Core
Summary: Flash 8 Player (Public Beta) crashes Deer Park → Flash 8 Player (Public Beta) crashes Deer Park [@ js_SetupLocks]
Version: unspecified → Trunk
QA Contact: general → plugins
bryner, sounds like this might be bfcache releated. Can you take a look? 

This is way down on the topcrash list for DPA2, unlike bug 300572 which is right
at the top.
Flags: blocking1.8b4? → blocking1.8b4-
Flags: blocking-aviary1.5?
Flags: blocking1.8b4?
Flags: blocking1.8b4-
Flags: blocking-aviary1.5?
Assignee: nobody → bryner
Flags: blocking1.8b4? → blocking1.8b4+
Bryner - looks like it might be BFCache related - can you take a look?
Does this bug bite only if bfcache is enabled
(browser.sessionhistory.max_viewers non-zero)?  There don't seem to be any
comments testifying to that.

This could be a Flash 8 bug.  It could be a bfcache bug, but comment 0 says
nothing about going back as in using the Back button -- step 2 says "restart and
go back to ..." but that's just talking about loading the same URL again in a
new browser instance.

We shouldn't bug bryner with this.  I'll look into it.

/be
Assignee: bryner → brendan
(In reply to comment #19)
> Does this bug bite only if bfcache is enabled
> (browser.sessionhistory.max_viewers non-zero)?  There don't seem to be any
> comments testifying to that.
 
Comment 12 does, and it's my experience as well that disabling bfcache makes the
Flash crasher go away.

> This could be a Flash 8 bug.  It could be a bfcache bug, but comment 0 says
> nothing about going back as in using the Back button -- step 2 says "restart and
> go back to ..." but that's just talking about loading the same URL again in a
> new browser instance.
> 
> We shouldn't bug bryner with this.  I'll look into it.
> 
> /be
(In reply to comment #12)
> (In reply to comment #7)
> > Does it also happen if you disable bfcache ?
> 
> I get a crash every time with bfcache to 3 (default);
> I get no crashes so far with bfcache to 0.
> 
> TB7498968H  TB7499076K  Sorry no links; the TB-server is too busy..

Duh, sorry I missed this -- I was find-as-you-type'ing "bfcache", not sure why
that didn't work.

Anyway, I'll look into this.  It sounds like some plugin API calling protocol
(maybe to do with SetWindow) got broken by the bfcache changes.  Cc'ing jst, and
if anyone knows of another bug about SetWindow regressions, please comment here.

/be
Target Milestone: --- → mozilla1.8beta4
Yes, it crashes only with bfcache enabled.
Need to dig more, hard to estimate ETA.  Will get together with jst for a plugin
API consultation and some hypotheses about the bug based on the crash traces
when he has more time, so next week.

/be
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [ETA 8/25?]
Sorry sorry, for being premature, the original testcase does not crash anymore
on my computer with Flash 8.0.15.0.

A similar new case (also bfcache) is discussed here:
http://forums.mozillazine.org/viewtopic.php?p=1685904#1685904
The talkback looks (for me) similar to the one I posted in comment #17.

Talkback: TB8524760Z  TB8529060Z

Testcase: go to http://www.macromedia.com/
Enter into the search box: beta
Click on Search button. 
Depends on: 156493
Stephend reproduced under VS.NET 2003 and showed the same stack as talkback:

<[1]stephend> > gkplugin.dll!_releaseobject(NPObject * npobj=0x04f420e8)  Line
1525 C++
<[1]stephend>   gkplugin.dll!NPObjWrapper_Finalize(JSContext * cx=0x0251ee08,
JSObject * obj=0x01c47db0)  Line 1195 + 0x6 C++
<[1]stephend>   js3250.dll!js_FinalizeObject(JSContext * cx=0x0251ee08, JSObject
* obj=0x01c47db0)  Line 2074 C
<[1]stephend>   js3250.dll!js_GC(JSContext * cx=0x0251ee08, unsigned int
gcflags=0)  Line 1839 + 0x9 C
<[1]stephend>   js3250.dll!js_ForceGC(JSContext * cx=0x0251ee08, unsigned int
gcflags=0)  Line 1510 + 0x1f C
<[1]stephend>   js3250.dll!JS_GC(JSContext * cx=0x0251ee08)  Line 1843 + 0x8 C
<[1]stephend>   gklayout.dll!nsJSContext::Notify(nsITimer * timer=0x029dcd10) 
Line 2156 C++
<[1]stephend>   xpcom_core.dll!nsTimerImpl::Fire()  Line 417 C++
<[1]stephend>   xpcom_core.dll!nsTimerManager::FireNextIdleTimer()  Line 628 C++
<[1]stephend>   gkwidget.dll!nsAppShell::Run()  Line 142 C++
<brendan> what is *obj
<stephend> - map 0x02dc6c88 {nrefs=1 ops=0x00cda480 _js_ObjectOps nslots=5 ...}
JSObjectMap *
<stephend>  nrefs 1 long
<stephend> + ops 0x00cda480 _js_ObjectOps JSObjectOps *
<stephend>  nslots 5 unsigned long
<stephend>  freeslot 4 unsigned long
<stephend> + slots 0x00ab9d94 long *
<brendan> stephend: *(JSClass*)(obj->slots[2]-1)
<stephend> - *(JSClass*)(obj->slots[2]-1) {name=0x00f36c38 "NPObject JS wrapper
class" flags=5 addProperty=0x00f2bcfd NPObjWrapper_AddProperty(JSContext *,
JSObject *, long, long *) ...} JSClass

Unfortunately, he couldn't get to the NPObjWrapper_Finalize or _releaseobject
frames, to confirm that obj's private data (of type NPObject) had already been
freed.  But that seems to be what happened.

This suggests the bug could be at least partly on the plugin's side.  If our
reference-counting of the NPObject is correct, then this looks like the plugin
double-dropped, or failed to hold when it should have.  Flash folks: is this
something you could check or double-check?  Thanks,

/be
When using bfcache we freeze the presshell when leaving a page, doing that ends
up tearing down the plugins in a window of time when a script global object is
not reachable from the document. Because of this, we won't find the JS context
for a plugin when we're invalidating the JS objects that wrap NPObjects from
the plugin, so we fail to clear their private data. The NPObjects are
invalidated, and thus freed etc, so we'll later on end up stomping all over
freed memory etc, which is likely to lead us to double releasing window objects
etc.

This fixes that problem by making GetJSContext() in nsJSNPRuntime.cpp use a
more reliable way to get to the JS context from a document.
Attachment #193458 - Flags: superreview?(brendan)
Attachment #193458 - Flags: review?(brendan)
Comment on attachment 193458 [details] [diff] [review]
Make sure we find cx so we can clear private data when destroying plugin 

sr=shaver, though I do wonder why doc->GetScriptGlobalObject doesn't do the
same thing as you're doing here.  I wonder idly, though; don't feel compelled
to explain!
Attachment #193458 - Flags: superreview?(brendan) → superreview+
Doesn't bfcache also set the container on the document to null, though?  So
we're depending on some sort of timing thing here?

It might be better for 1.9 to move ahead with making GetScriptGlobalObject never
return null and then backing out this change, since at that point using
GetScriptGlobalObject will be far less fragile.
(In reply to comment #27)
> (From update of attachment 193458 [details] [diff] [review] [edit])
> sr=shaver, though I do wonder why doc->GetScriptGlobalObject doesn't do the
> same thing as you're doing here.  I wonder idly, though; don't feel compelled
> to explain!

Just to put the record staight, the reason this happens is that we end up
calling SetScriptGlobalObject(nsnull) on the document when we leave the page,
then we call Freeze() on the pres shell, and that's where we end up tearing down
the plugin stuff. After this, the document's mScriptGlobalObject is null, but
mIsGoingAway false, so GetScriptGlobalObject() will return null.
Comment on attachment 193458 [details] [diff] [review]
Make sure we find cx so we can clear private data when destroying plugin 

r+a=me, thanks for fixing this.  Leave the bug open for a better trunk patch,
or file a followup bug?

/be
Attachment #193458 - Flags: review?(brendan)
Attachment #193458 - Flags: review+
Attachment #193458 - Flags: approval1.8b4+
Assignee: brendan → jst
Status: ASSIGNED → NEW
Whiteboard: [ETA 8/25?] → [ETA 8/22]
No longer depends on: 300572
*** Bug 300572 has been marked as a duplicate of this bug. ***
Fixed on the branch.
Status: NEW → ASSIGNED
Keywords: fixed1.8
This checkin seems to have caused a regression in the behavior of Adblock and
the unofficial Adblock plus extensions.

Using the following testcase:
Go to www.macromedia.com
Entered into the search box: beta
Click "Search"
hit back button

All flash objects have now disappeared on the front page of macromedia.com

Disabling Adblock from the extension manager fixes this. Please note that the
Adblock filter in use (Filterset.G) is not blocking the flash objects on this page.

Further commentary here:
http://forums.mozillazine.org/viewtopic.php?p=1693080#1693080
That's VERY unlikely to be a result of this checkin.  What's the date range on
that regression?
And I would recommend filing a separate bug on the regression.
We also had to fix some Flash Player issues in order to fix this bug. The fixes
are in Flash Player 8r19, which is not yet publicly available.
*** Bug 303811 has been marked as a duplicate of this bug. ***
Whiteboard: [ETA 8/22] → [ETA 8/22][has approval]
(In reply to comment #28)
> Doesn't bfcache also set the container on the document to null, though?  So
> we're depending on some sort of timing thing here?
> 
> It might be better for 1.9 to move ahead with making GetScriptGlobalObject never
> return null and then backing out this change, since at that point using
> GetScriptGlobalObject will be far less fragile.

We should do this, but if not soon, then I say land the branch patch on the
trunk to avoid the crash and talkback noise.  jst, what do you say?

/be
Flags: blocking1.8b5+
In case I was unclear, I'm all for fixing this bug on trunk with the patch we
have.  As long as we file a followup bug to make the world a better place.
OK, I checked in that patch on the trunk and filed bug 308902 on the issue of
mScriptGlobalObject immutability.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
I've run the full gamut of my usual Flash-powered websites, and have had _zero_
Flash crashes using:

SeaMonkey 1.1a:Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1)
Gecko/20050917 Mozilla/1.0

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050917
Firefox/1.6a1

Verified FIXED on trunk; thanks!
Status: RESOLVED → VERIFIED
v.fixed on branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5)
Gecko/20050928 Firefox/1.4 and Shockwave Flash 8.0 r22, no crashes with urls in
this bug and other Flash content I see daily.
Keywords: fixed1.8verified1.8
*** Bug 304351 has been marked as a duplicate of this bug. ***
No longer depends on: 156493
Crash Signature: [@ js_SetupLocks]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.