Crash closing a window with a plugin [@ nsObjectFrame::NotifyContentObjectWrapper]

RESOLVED WORKSFORME

Status

()

Core
Plug-ins
--
critical
RESOLVED WORKSFORME
13 years ago
2 years ago

People

(Reporter: timeless, Unassigned)

Tracking

({crash})

Trunk
PowerPC
Mac OS X
crash
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8rc1 -
blocking1.8.1 -
blocking1.8.0.1 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ccbr], crash signature, URL)

Attachments

(4 attachments)

(Reporter)

Description

13 years ago
Incident ID: 10505072
Stack Signature	nsObjectFrame::NotifyContentObjectWrapper() 9dbc11cc
Product ID	Camino10
Build ID	2005091409
Trigger Time	2005-10-10 23:40:04.0
Platform	MacOSX
Operating System	Darwin 8.2.0
Module	Camino + (00510790)
URL visited	
User Comments	
Since Last Crash	259 sec
Total Uptime	259 sec
Trigger Reason	SIGBUS: Bus Error: (signal 10)
Source File, Line No.
/builds/camino-release/camino/1.0/mozilla/layout/generic/nsObjectFrame.cpp, line 623
Stack Trace 	
nsObjectFrame::NotifyContentObjectWrapper() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsObjectFrame.cpp,
line 623]
nsObjectFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsObjectFrame.cpp,
line 1267]
nsLineLayout::ReflowFrame() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsLineLayout.cpp, line
995]
nsBlockFrame::ReflowInlineFrame() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
4002]
nsBlockFrame::DoReflowInlineFrames() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
3840]
nsBlockFrame::ReflowInlineFrames() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
3710]
nsBlockFrame::ReflowLine() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
2707]
nsBlockFrame::ReflowDirtyLines() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
2241]
nsBlockFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
879]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
nsTableCellFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableCellFrame.cpp,
line 441]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
nsTableRowFrame::ReflowChildren() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableRowFrame.cpp,
line 961]
nsTableRowFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableRowFrame.cpp,
line 1405]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
nsTableRowGroupFrame::ReflowChildren() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableRowGroupFrame.cpp,
line 390]
nsTableRowGroupFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableRowGroupFrame.cpp,
line 1242]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
nsTableFrame::ReflowChildren() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableFrame.cpp, line
3197]
nsTableFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableFrame.cpp, line
1909]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
nsTableOuterFrame::OuterReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableOuterFrame.cpp,
line 1315]
nsTableOuterFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/tables/nsTableOuterFrame.cpp,
line 1968]
nsBlockReflowContext::ReflowBlock() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockReflowContext.cpp,
line 189]
nsBlockFrame::ReflowBlockFrame() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
3430]
nsBlockFrame::ReflowLine() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
587]
nsBlockFrame::ReflowDirtyLines() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
2241]
nsBlockFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
879]
nsBlockReflowContext::ReflowBlock() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockReflowContext.cpp,
line 189]
nsBlockFrame::ReflowBlockFrame() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
3430]
nsBlockFrame::ReflowLine() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
587]
nsBlockFrame::ReflowDirtyLines() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
2241]
nsBlockFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
879]
nsBlockReflowContext::ReflowBlock() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockReflowContext.cpp,
line 189]
nsBlockFrame::ReflowBlockFrame() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
3430]
nsBlockFrame::ReflowLine() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
587]
nsBlockFrame::ReflowDirtyLines() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
2241]
nsBlockFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsBlockFrame.cpp, line
879]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
CanvasFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsHTMLFrame.cpp, line 520]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
nsHTMLScrollFrame::ReflowScrolledFrame() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 578]
nsHTMLScrollFrame::ReflowContents() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 568]
nsHTMLScrollFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 760]
nsContainerFrame::ReflowChild() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsContainerFrame.cpp,
line 919]
ViewportFrame::Reflow() 
[/builds/camino-release/camino/1.0/mozilla/layout/generic/nsViewportFrame.cpp,
line 241]
IncrementalReflow::Dispatch() 
[/builds/camino-release/camino/1.0/mozilla/layout/base/nsPresShell.cpp, line 52]
PresShell::ProcessReflowCommands() 
[/builds/camino-release/camino/1.0/mozilla/layout/base/nsPresShell.cpp, line 6870]
ReflowEvent::HandleEvent()
PL_HandleEvent() 
[/builds/camino-release/camino/1.0/mozilla/xpcom/threads/plevent.c, line 689]
PL_ProcessPendingEvents() 
[/builds/camino-release/camino/1.0/mozilla/xpcom/threads/plevent.c, line 623]
__CFRunLoopDoSources0()
__CFRunLoopRun()
CFRunLoopRunSpecific()
RunCurrentEventLoopInMode()
ReceiveNextEventCommon()
BlockUntilNextEventMatchingListInMode()
_DPSNextEvent()
-   -   NSApplicationMain()
nsDeque::Last()   nsDeque::Peek()

Comment 1

13 years ago
This is a top crasher for Camino, and occurs in Firefox.
Flags: blocking1.8rc1?

Comment 2

13 years ago
Way down the list at #115 for Firefox 1.5b1 and #206 for 1.5b2. Not gonna block
the release on this.
Flags: blocking1.8rc1? → blocking1.8rc1-

Comment 3

12 years ago
#3 topcrash for camino. we need this.
Flags: blocking1.8.1?
The last time Camino sees this crash is on the 11/18 branch build.
Summary: [@ nsObjectFrame::NotifyContentObjectWrapper] → crash [@ nsObjectFrame::NotifyContentObjectWrapper]

Comment 5

12 years ago
That's because we don't have any symbols.

F.
who's the right person to look at this? this is prolly a 1.0 stopper for camino.
Flags: blocking1.8.0.1?
So does anyone know _why_ this is happening?  Dealing with this stuff on branch is pretty hellish at this point, at least for me -- I don't recall very well how it works there anymore.
If you get a tested reviewed patch you can seek approval but not blocking 1.8.0.1 (a non-nobody assignee would be good if you're serious about getting this one into a release)
Flags: blocking1.8.0.1? → blocking1.8.0.1-

Comment 9

12 years ago
We really need repro steps for this.
Whiteboard: needinfo

Comment 10

12 years ago
I can repro this now. Go to
http://www.cur1350.co.uk/index.php?section=1
and click on the "listen now" graphic near the top. As soon as you see the poup window contents paint, close the window (I used command-W). This will crash.
Assignee: nobody → sfraser_bugs

Updated

12 years ago
Summary: crash [@ nsObjectFrame::NotifyContentObjectWrapper] → Crash closing a window with a plugin [@ nsObjectFrame::NotifyContentObjectWrapper]
Simon, why do we actually crash there, can you tell?  Is the frame dead or something?

Comment 12

12 years ago
I haven't had a chance to debug much, and Java's exception handlers (which kick in on the crash) may be messing with gdb's varaibles display, but when I caught it in gdb, the frame looked like garbage.

Comment 13

12 years ago
Assertions preceding the crash:

###!!! ASSERTION: about to crash due to bug 136927: '!mInstantiating', file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 712
Break: at file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 712
nsObjectFrame 0x2a277c8 dtor
--WEBSHELL 0x1dcf45a0 == 2
[Switching to process 849 thread 0x23003]
2006-01-20 07:35:28.187 Camino[849] Browser controller died.
2006-01-20 07:35:28.208 Camino[849] The browser wrapper died.
2006-01-20 07:35:28.210 Camino[849] CHBrowserView died.
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
For application/x-java-vm found plugin MRJPlugin.plugin
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3126
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2448
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3126
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2782
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3126
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2448
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3126
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2782
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(GetTagType(&tagType))) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3045
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3126
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2448

So this might be a dup of bug 136927.
Depends on: 136927
Whiteboard: needinfo

Comment 14

12 years ago
I'm still crashing even with the 1.8 branch patch from bug 136927:

###!!! ASSERTION: invalid active window: 'Error', file ../../../../../../mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp, line 924
Break: at file ../../../../../../mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp, line 924
2006-01-20 07:56:55.682 Camino[1254] Browser controller died.
2006-01-20 07:56:55.705 Camino[1254] The browser wrapper died.
2006-01-20 07:56:55.706 Camino[1254] CHBrowserView died.
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
For application/x-java-vm found plugin MRJPlugin.plugin
###!!! ASSERTION: nsPluginHostImpl not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
Break: at file ../../../../../../mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp, line 2594
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3194
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2516
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3194
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2850
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3194
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2516
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3194
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2850
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(GetTagType(&tagType))) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3113
WARNING: NS_ENSURE_TRUE(mOwner) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 3194
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file ../../../../mozilla/layout/generic/nsObjectFrame.cpp, line 2516

Stack:

#0  0x068f7ee4 in nsObjectFrame::NotifyContentObjectWrapper (this=0x2a91fc8) at ../../../../mozilla/layout/generic/nsObjectFrame.cpp:2205
#1  0x068f4d70 in nsObjectFrame::HandleInstantiateEvent (this=0x2a91fc8) at ../../../../mozilla/layout/generic/nsObjectFrame.cpp:1271
#2  0x068f4e18 in nsObjectFrame::InstantiateEvent::Handle (self=0x1fe2bbd0) at ../../../../mozilla/layout/generic/nsObjectFrame.cpp:1288
#3  0x0188bb18 in PL_HandleEvent (self=0x1fe2bbd0) at ../../../../mozilla/xpcom/threads/plevent.c:688
#4  0x0188b930 in PL_ProcessPendingEvents (self=0x2663770) at ../../../../mozilla/xpcom/threads/plevent.c:623
#5  0x0188c104 in _md_EventReceiverProc (info=0x2663770) at ../../../../mozilla/xpcom/threads/plevent.c:1559
#6  0x9075db20 in __CFRunLoopDoSources0 ()
#7  0x9075cf98 in __CFRunLoopRun ()
#8  0x9075ca18 in CFRunLoopRunSpecific ()
#9  0x9318e1e0 in RunCurrentEventLoopInMode ()
#10 0x9318d7ec in ReceiveNextEventCommon ()
#11 0x9318d6e0 in BlockUntilNextEventMatchingListInMode ()
#12 0x9368c104 in _DPSNextEvent ()
#13 0x9368bdc8 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#14 0x9368830c in -[NSApplication run] ()
#15 0x93778e68 in NSApplicationMain ()
Um.. what's with the threadsafety stuff?  If people are poking layout from off-thread, all bets are off...

Comment 16

12 years ago
I think that's just Java doing stupid crap. I can crash without those showing up.

With the patch for 136927, I'm still seeing NotifyContentObjectWrapper being called on a deleted nsObjectFrame (via the plevent).
So the nsObjectFrame is 0xdddddddd there?  That is, all its members are that?

Is revoking the event in nsObjectFrame::Destroy failing somehow?

Comment 18

12 years ago
Created attachment 209179 [details]
nsObjectFrame dtor stack

This stack trace shows JEP doing some really bad stuff. Down under nsObjectFrame::InstantiatePlugin(), we're firing up the Java plugin and the JVM, and as part of that JEP is running an event loop ([NSApplication sendEvent:]), which happens to catch the Command-W event, which proceeds to tear down the window, including the nsObjectFrame on the stack. Ick.

However, we've had crash reports for Flash pages, so it can't just be JEP that's at fault here.

Comment 19

12 years ago
The JEP code
http://cvs.sourceforge.net/viewcvs.py/javaplugin/JavaEmbeddingPlugin/AppletView.m?rev=1.9&view=markup

has:

    while (!awtLibrariesLoaded) {
        anEvent = [NSApp nextEventMatchingMask:NSAnyEventMask
            untilDate:[NSDate dateWithTimeIntervalSinceNow:.1]
            inMode:NSDefaultRunLoopMode dequeue:YES];
        if (anEvent) {
            [NSApp performSelector:@selector(_setCurrentEvent:) withObject:anEvent];
            [NSApp sendEvent:anEvent];
        }
    }

which I think is very bad to do; it can mess with the application's event handling in all sorts of ways.
> This stack trace shows JEP doing some really bad stuff.

No.

The JEP has just called JNI_CreateJavaVM() (which initiates a bunch of
activity on secondary threads), and is waiting for the AWT libraries to
finish loading before proceeding.  This is necessary in order for Apple's JVM
to be launched on demand (instead of as the browser starts up).

Because this delay is happening on the main thread, the JEP needs to process
main thread events while it's waiting.

See AppletView maybeCreateJavaVM in AppletView.m.

> However, we've had crash reports for Flash pages, so it can't just be JEP
> that's at fault here.

Sure, but those could well be bug 136927.

Steven, we understand why the JEP is processing events.  Thing is, processing events during reflow is pretty much guaranteed to lead to crashes -- see the old bugs on the image loading prompt and the issues it led to.  And on the 1.8 branch plugin instantiation happens during reflow.... (not that this actually ends up being a problem; see further).

With the patch for bug 136927, we shouldn't be instantiating during reflow, at least.  But what we _do_ end up doing is getting destroyed under InstantiateEmbeddedPlugin.  And then when that returns, we call NotifyContentObjectWrapper, which is where we're crashing (since we're dead).

I'd think this would even be an issue on trunk, since on trunk we append a reflow command for the object frame _after_ we InstantiateEmbeddedPlugin()... so after the frame is dead.

So the basic issue is that event processing of any sort is absolutely not OK while we're in frame code.  :(

Perhaps in addition to the changes in bug 136927 we could move the NotifyContentObjectWrapper() call to something that outlives instantiation here?  Like the instantiate event?  Note that all it needs is the content node off the frame, which it can get and store in an nsCOMPtr<nsIContent> before calling HandleInstantiateEvent.

Comment 22

12 years ago
Created attachment 209181 [details] [diff] [review]
Attachment 206646 [details] [diff] from bug 136927 modified to avoid the crash

This patch (diff -w) is the same as attachment 206646 [details] [diff] [review] with a change to make NotifyContentObjectWrapper() a static method, to which the mContent is passed in (keeping it alive with an nsCOMPtr on the stack). This allows it to not crash if |this| has been deleted inside InstantiatePlugin.
Attachment #209181 - Flags: superreview?(bzbarsky)
Attachment #209181 - Flags: review?

Updated

12 years ago
Attachment #209181 - Flags: review? → review?(dbaron)
Comment on attachment 209181 [details] [diff] [review]
Attachment 206646 [details] [diff] from bug 136927 modified to avoid the crash

The patch this was based on wasn't anywhere near ready for review; for a start, it broke full page plugins.  Beyond that, it had very little testing.

I see no indication that this fixes the full-page-plugins issues.  Second, I see no sign that it has the level of testing needed to take on the branch, and it probably is a branch-only patch due to the significant changes that have happened to this code on the trunk.  So I'm not sure what the point of reviewing it is, and I'm marking it r-.
Attachment #209181 - Flags: review?(dbaron) → review-
What would be worth reviewing would be a trunk patch to fix this bug, which would probably look a lot like the diffs between the two patches.

Comment 25

12 years ago
(In reply to comment #23)
> (From update of attachment 209181 [details] [diff] [review] [edit])
> The patch this was based on wasn't anywhere near ready for review; for a start,
> it broke full page plugins.  Beyond that, it had very little testing.

Sorry, I failed to read all of bug 136927 before filching this patch ("patch by dbaron! it must be good!").
(In reply to comment #21)

> Steven, we understand why the JEP is processing events.

This wasn't obvious :-)

> But what we _do_ end up doing is getting destroyed under
> InstantiateEmbeddedPlugin.  And then when that returns, we call
> NotifyContentObjectWrapper, which is where we're crashing (since we're
> dead).

> So the basic issue is that event processing of any sort is absolutely not OK
> while we're in frame code.  :(

So you're saying that the JEP (or some other plugin) doing event processing is
responsible for "getting destroyed under InstantiateEmbeddedPlugin"?

> So you're saying that the JEP (or some other plugin) doing event processing is
> responsible for "getting destroyed under InstantiateEmbeddedPlugin"?

Yes.

Comment 28

12 years ago
(In reply to comment #26)

> So you're saying that the JEP (or some other plugin) doing event processing is
> responsible for "getting destroyed under InstantiateEmbeddedPlugin"?

Look at the stack in attachment 209179 [details].

Comment 29

12 years ago
Created attachment 209217 [details]
Another stack (applet loading) showing frame destruction under InstantiatePlugin

Steven: here's another case of event handling (this time when the awt libs are already loaded) causing frame teardown inthe middle of plugin instantiation.

Comment 30

12 years ago
I spun off bug 324281 on the JEP issues.
Depends on: 324281
Created attachment 209239 [details]
Stack traces of non-frame-code crashes

This is a strange bug.  Simon's procedure from comment #10 will often
trigger a crash, but his description is incomplete -- the crash
doesn't happen until the _next_ thing you do somewhere in Camino's UI.

You can get lots of different kinds of crashes, depending on what this
"next thing" is, and how long you wait to do it.  Most of these
crashes aren't in frame code (and don't take place while the JEP is
processing events).

I'm appending two crash logs in one file.  In both cases I waited
about 5 seconds before doing anything else, after having followed
Simon's procedure.  In the first case the "next thing" was to try to
access the "Camino" menu.  In the second case the "next thing" was to
click on the red close button to close the window displaying the
http://www.cur1350.co.uk/index.php?section=1 URL (the window from
which the popup window (now vanished) had popped up).

Comment 32

12 years ago
Let's put the JEP-related comments in bug 324281.
You guys (Boris and Simon) have shown that a "window close" message can be
processed while the JEP is processing events on the main thread, while frame
code is running -- causing the plugin instance to be destroyed while frame
code is running, leading to a crash.

You haven't shown that this is the _only_ way that "window close" (or other
similarly disruptive events) can be processed while frame code is running.  I
need to run some tests with an altered version of the JEP (one which doesn't
ever process events), to see what I find out.  But in the meantime I'll assume
that this is true.

However, even if it's true that the JEP's event processing can trigger frame
code crashes that wouldn't otherwise happen, I wonder how likely this is (how
often it happens).

Some of the JEP's event processing can probably be done away with -- for
example what it does while waiting for the AWT libraries to load while the JVM
is launching.  Though I'll have to test this to be sure.  But I already know
that the event processing it does while an applet is being loaded is
necessary, and can't be done away with.  So I'd like to know if it's really
worth the trouble to pursue the kind of workarounds being discussed at bug
324281.

> Let's put the JEP-related comments in bug 324281.

Comment #31 isn't JEP-related.  Comment #33 asks an important question related
to this bug (how often is the JEP really involved?).  But from here on I'll
post my JEP-specific comments at bug 324281.
(Following up comment #31)

I've found that I _can_ make Simon's procedure work, as it stands (without an
extra step), as long as I use Command+W early enough.  But when doing this I
_always_ get the crash in nsObjectFrame::NotifyContentObjectWrapper() that
timeless originally reported.  I _never_ see anything like the traces that
Simon posted.

...

But Simon now tells me that the traces he posted weren't crash traces.  See:

https://bugzilla.mozilla.org/show_bug.cgi?id=324281#c3
https://bugzilla.mozilla.org/show_bug.cgi?id=324281#c4

I'll see what I can make of all this tomorrow :-)

Comment 36

12 years ago
(In reply to comment #31)
> Created an attachment (id=209239) [edit]
> Stack traces of non-frame-code crashes
> 
> This is a strange bug.  Simon's procedure from comment #10 will often
> trigger a crash, but his description is incomplete -- the crash
> doesn't happen until the _next_ thing you do somewhere in Camino's UI.

This is typical of an Obj-C retain count bug, where autorelease happens at event loop time. You should run with NSZombieEnabled=YES in your environment to try to track these crashes down (I think we see a fair number of them in Talkback, and I suspect that JEP is responsible for some fraction of those).
I've just released version 0.9.5+c of the Java Embedding Plugin, which largely
resolves this bug.  I found a workaround that allows the JEP to process events
when it needs to, without (mostly) allowing Mozilla.org frame code to be
reentered.  (I say "mostly" because I couldn't get my workaround to work on OS
X 10.2.8.  So on that OS Camino is still vulnerable to this bug.)

http://javaplugin.sourceforge.net/

(In reply to comment #36)

My workaround doesn't get rid of the problems I described in comment #31,
which I'm now pretty sure have nothing to do with the JEP.  I did try setting
NSZombieEnabled a couple of times on OS X 10.3.9 and 10.4.4.  _NSZombie never
displayed any messages in those tests -- which shows (I think) that messages
weren't being sent to deleted objects.  Instead, I always got (in addition to
the crash logs) a message in the console that the stack had been overflowed.

Updated

12 years ago
Depends on: 325147

Comment 38

12 years ago
(In reply to comment #37)
> I've just released version 0.9.5+c of the Java Embedding Plugin, which largely
> resolves this bug.  I found a workaround that allows the JEP to process events
> when it needs to, without (mostly) allowing Mozilla.org frame code to be
> reentered.

I tested this new version, and it appears to work as advertised. Thanks Steven! Now we just have to get it checked in; I filed bug 325147.
No longer depends on: 325147
*** Bug 338148 has been marked as a duplicate of this bug. ***

Comment 40

12 years ago
Not going to block 1.8.1 for this bug, but we'll happily consider a baked-on-trunk patch.
Flags: blocking1.8.1? → blocking1.8.1-
This bug (312062) and bug 324281 should probably just be marked
RESOLVED FIXED ... though the problem isn't fixed on OS X 10.2.8 (and
probably never will be).

See comment #37, comment #38 and bug 324281 comment #21.
I crash with this stacktrace in this example:
http://wargers.org/mozilla/bug312062/alert.htm
First close the Firefox/Mozilla window, then close the alert. The crash only happens on branch, not on trunk.
Martijn, I tried your link (http://wargers.org/mozilla/bug312062/alert.htm) with
the latest 1.8 branch (BonEcho) nightly (2006-07-26-04-mozilla1.8), but I didn't
see any "alert" or get a crash.

I've got version 2.1 of Manfred Schubert's PDF Browser Plugin installed, which I
think is the latest version.

(Your link, as it now stands, loads a PDF document.)
Ah, yeah, sorry, I should have told which plugin I used.
I used Adobe Reader 7.0.0 in which I saw the crash. I now upgraded to Adobe Reader 7.0.7, and now I don't get a crash and the embed is working fine now.
So I guess it depends on what weird stuff the plugin is doing, to get this crash.
I just crashed in this tonight in today's 1.8 branch Camino nightly (JEP 0.9.6) on 10.3.9 with an applet loading in one window while I closed another window with a different applet.  Unfortunately, the site in question is behind several logins :(
Smokey, your report had me scratching my head for a while.

But, after digging through my code, I remembered that I'd (slightly)
loosened my workaround for this bug, so that (now) the workaround is
no longer active for Cocoa apps (i.e. Camino) on OS X 10.3.X.

So those who run Camino on 10.3.X will (like those who run any
Mozilla.org browser on OS X 10.2.8) sometimes see this bug.  And those
who run Carbon-based apps (like Firefox and Seamonkey) on 10.3.X
should (like those who run any Mozilla.org browser on OS X 10.4.X or
higher) never see it (or if they do see it, it won't have been
triggered by the JEP).

I'm a bit embarrassed to say that I don't quite remember why I made
this change.  I was working at a white heat on the JEP 0.9.6 update,
and do remember that I saw a reproducable hang that my change fixed
... but I neglected to write all this down :-(

OS X 10.3 is, of course, getting older by the minute :-)

But if this bug once again starts to be a problem to lots of people, I
can undo my change.  Though this will (of course) reintroduce the
_other_ problem.
(Following up comment #46)

I still haven't been able to reconstruct/recall the reproducible hang
that I mentioned in comment #46.  Nor have I run across it again
accidentally.

I've noticed a few (6) Talkback logs (since the beginning of 2007) for
crashes in nsObjectFrame::NotifyContentObjectWrapper on Camino
nightlies that (by their date) must bundle JEP 0.9.6.  All took place
on OS X 10.3.9, and so were probably triggered by the change I
mentioned in comment #46.

So I'm going to undo my change -- I'm going to re-enable my workaround
for Cocoa browsers on OS X 10.3.X.  (The workaround will stay disabled
for all browsers on OS X 10.2.8, will stay enabled for all browsers on
OS X 10.4.X and higher, and will stay enabled for Carbon browsers on
OS X 10.3.X.)

This change will appear in JEP 0.9.6.1, which I hope to release within
a week.

Talkback also had some other interesting data.  I didn't see any
crashes in nsObjectFrame::NotifyContentObjectWrapper in Firefox on any
version of OS X.  But I noticed a very large number on Windows and
Linux.  I also noticed a few crashes on OS X 10.3.9 in versions of
Camino (1.0 through 1.0.3) that bundle JEP versions that contain (and
enable) my workaround for Cocoa browsers on 10.3.X.

This tends to confirm what I've long suspected -- bug 312062 is caused
by bugs and/or design flaws in Mozilla.org browsers, not by the JEP.
All the JEP does (when my workaround is disabled) is to trigger these
crashes.
I've just released a new version (0.9.6.1) of the Java Embedding
Plugin that implements a new workaround for this bug -- one that works
in both Carbon and Cocoa-based browers and in all versions of OS X
from 10.2.8 up.  For more information see bug 376395.

It didn't take me long to bump into the "reproducible hang" that I
talked about in comment #47 -- this happened almost immediately after
I "undid my change".  For some reason these hangs don't happen (or
happen only too rarely) with JEP 0.9.5+g (and previous).  So now I was
in a quandary.

But necessity is the mother of invention, and after some digging
around I discovered my new (and improved) workaround.  For a more
detailed discussion see item #10 in JEP 0.9.6.1's Changes.txt
(attached at bug 376395).

Updated

9 years ago
Assignee: sfraser_bugs → nobody

Comment 49

9 years ago
This is no longer a topcrash, probably thanks to Steven's workaround for JEP (Java).

It sounds like this is a well-understood problem in Gecko's plugin code.  Steven and Josh, will you be able to fix it?
Keywords: topcrash
Whiteboard: [ccbr]

Comment 50

9 years ago
(Oh, and updating the summary to reflect the code-level issue would be nice.)
> It sounds like this is a well-understood problem in Gecko's plugin
> code.

I now think the only way to *really* fix this kind of bug is to load
plugins asynchronously -- to start loading a given plugin object, and
then have the plugin call a callback to tell the browser that the
object has finished loading.  Doing this would mean that the JEP no
longer has to spin the main thread's event loop as it loads the JVM or
a plugin.

> Steven and Josh, will you be able to fix it?

I doubt this will ever happen for in-process plugins ... though I
could be wrong.  Out-of-process plugins will probably need to be
loaded asynchronously.  So once that's implemented, it might be
possible to back-port that capability to in-process plugins.  If we
still even support in-process plugins :-)
FWIW, we still have a rash of these crashes in Camino 1.6.x (it's the top crash actually in Mozilla code) on 10.4.11 and 10.5.7 (so plugins other than the JEP are still triggering it there).  However, something in Gecko 1.9 (reflow branch?) seems to have helped mitigate the crash to the point that I've only seen one or two reports since we switched to Breakpad.

Comment 53

9 years ago
Would a fix for bug 90268 / bug 136927 take care of this bug, or is there something else going on here?
Keywords: testcase-wanted
(Assignee)

Updated

7 years ago
Crash Signature: [@ nsObjectFrame::NotifyContentObjectWrapper]
No recent reports of crashes with this stack, and the plugin code has changed significantly since it was filed to the point where any new crashes should be in a new bug anyway.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Keywords: testcase-wanted
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.