324281 - Java Embedding Plugin's (JEP) handling of user events while loading applets causes crashes

Status: RESOLVED FIXED

(Plugins Graveyard :: Java (Java Embedding Plugin), defect)

Description

[Simon Fraser [no longer active]]

Spun off from bug 312062, so that we can focus on JEP-related solutions here.

The problem is that JEP runs an event loop while waiting for AWT libraries to load, and while doing async applet loading. This is problematic because user events that get handled in that event loop can cause the browser window hosting the plugin to get torn down (e.g. as a result of the user typing Command-W). This is a very common type of crash in Camino and Firefox.

Comment 1

[Simon Fraser [no longer active]]

Here are some ways we could approach this problem:

1. Avoid having JEP do any event handling while waiting for awt/applet loading.
   Is this do-able? Does JEP have to be running an event loop so that stuff like
   security dialog display works?

2. Have JEP only handle a subset of event types, or filter events to avoid user
   events getting processed (again, security dialogs may make this hard). Or
   maybe do something with event loop modes.

3. Have JEP send out a notification before and after running one of these
   event loops, which Camino can use to disable all its menu items. Not sure
   how we'd fix Firefox to do something similar.

4. Have JEP add a method to MyNSApplication that Camino can call to tell if
   such an event loop is running.

Comment 2

[Simon Fraser [no longer active]]

I'd test some of these theories out if I could get Camino to load a JEP that I built, but for some reason it doesn't want to (even though applets still work). I'm thorougly confused.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

I simply can't reproduce the crashes for which you gave logs at bug 312062,
and on which the necessity for this bug is based:

https://bugzilla.mozilla.org/attachment.cgi?id=209179
https://bugzilla.mozilla.org/attachment.cgi?id=209217

All I get are crashes in nsObjectFrame::NotifyContentObjectWrapper(), like the
one timeless originally reported.

This happens even when I change the JEP code to post a Command+W event just
before it starts processing events in AppletView maybeCreateJavaVM, waiting
for the AWT libraries to finish loading.

The popup window _does_ close.  By tracing NSApplication sendEvent, I see that
the JEP _is_ processing my fake keyDown event.  And by adding traces to
NSApplication _handleKeyEquivalent and NSWindow close, I see that they _are_
being called at the same time (as happens in your stack traces), while Camino
is still executing frame code.

But the crashes always come later, after the JEP has stopped processing events
in maybeCreateJavaVM (and before it starts processing them again in AppletView
initWithParams).

I tested with Camino 1.0b2 and with the latest 1.0 branch nightly
(2006-01-21-04-1.0), on both OS X 10.4.4 and 10.3.9.

Which Camino version(s) have you been using, on which Mac OS X versions?

Comment 4

[Simon Fraser [no longer active]]

(In reply to comment #3)
> I simply can't reproduce the crashes for which you gave logs at bug 312062,
> and on which the necessity for this bug is based:
> 
> https://bugzilla.mozilla.org/attachment.cgi?id=209179
> https://bugzilla.mozilla.org/attachment.cgi?id=209217
> 
> All I get are crashes in nsObjectFrame::NotifyContentObjectWrapper(), like the
> one timeless originally reported.

Those are not crashes. Those are stack traces from the debugger that show _why_ we crash later in NotifyContentObjectWrapper (br on nsObjectFrame::~nsObjectFrame), because we're blowing away the nsObjectFrame while its code is running on the stack.

The point of those stack traces is to show the cause of the problem: that Gecko assumes that plugin instantiation won't do anything that might cause Gecko to be re-entered (like tearing down the entire window that is hosting the plugin), but that the JEP allows this to happen (by handling events).

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Alright, I've convinced myself that the JEP's event processing is the
proximate cause of all the crashes with the particular stack trace that
timeless reported at bug 312062.

Here's what I did:

1) I altered JavaEmbeddingPlugin.bundle to use NSApplication postEvent to post
   a fake NSKeyDown event corresponding to Command+W just before the JEP
   started processing events in AppletView maybeCreateJavaVM.  When I used
   this altered JEP to load bug 312062's URL, I saw timeless's crash.

2) I altered the JEP again to post a fake Command+W event just before the JEP
   started processing events in AppletView initWithParams, and again got
   timeless's crash loading the bug 312062 URL.

3) Following up on step 2, I also disabled the JEP's event processing in
   initWithParams.  Now when I tried loading the bug 312062 URL, the popup
   window still closed prematurely, but I no longer got a crash.  (It wasn't
   possible to follow up step 1 the same way -- disabling the JEP's event
   processing in maybeCreateJavaVM causes the JEP (and the browser) to hang.)

There are other reports of crashes in
nsObjectFrame::NotifyContentObjectWrapper(), in which the JEP doesn't seem to
be involved (some of them have been marked "fixed").  But none of their
signatures exactly match that of bug 312062.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #1)

There are actually three places in which the JEP can process events on the
main thread (the two already mentioned here, plus another one that isn't
relevant to this particular bug).  In none of these cases can the JEP's event
processing be disabled.

1) In AppletView maybeCreateJavaVM the JEP processes events on the main thread
   while it's waiting for the AWT libraries to finish loading.

   If the JEP doesn't wait for the AWT libraries to load, it will hang as
   (subsequently) an applet gets loaded in AppletView initWithParams.  (The
   only way around this is to load the JVM as the browser starts up (which is
   what the JEP actually used to do).  But since the "fix" for bug 128366,
   this is no longer possible.)

   If the JEP does wait here for the AWT libraries to load, but doesn't
   process events on the main thread, the JEP will hang waiting for the AWT
   libraries to load.

2) In AppletView initWithParams the JEP processes events on the main thread
   while it waits for the applet to be loaded and sufficiently initialized
   (enough that it will be possible for the applet's methods to be called via
   LiveConnect from a JavaScript OnLoad handler).

   If the JEP doesn't wait for the applet to load and be sufficiently
   initialized, there will be various problems (including LiveConnect not
   working from a JavaScript OnLoad handler).

   If (as Simon noted) the JEP doesn't process events while it's waiting, it
   won't be possible to interact with any dialogs the JVM puts up as the
   applet is loading (such as those you see with signed applets).

3) In Java_processMainThreadEvents() (in AppletView.m) the JEP processes
   events on the main thread when called from a couple of different places in
   Java code, currently only on Mac OS X Tiger.

   a) AppletHolder.stopApplet(), where the JEP calls
      AppletHolder.stopAppletDirectly() on another thread (that of the main
      AWT event queue), and waits on the main thread for that call to finish.

      If the JEP doesn't wait here, the JEP will crash.  If it waits without
      processing events, the JEP will hang.  If it simply calls
      stopAppletDirectly() on the same thread (the main thread), again the JEP
      will sometimes hang.

   b) LiveConnect.doProxy() (called from JEPDoLiveConnectProxy() in
      Controller.m), which posts a JavaScript-to-Java LiveConnect call to an
      event queue running on a secondary thread, and waits on the main thread
      for the call to finish.

      If the JEP doesn't wait here, one LiveConnect command will start being
      executed before the previous one has finished.  If it doesn't process
      events, the JEP will hang.

  (There's also a Java_processQueuedMainThreadEvents(), but that currently
  isn't used.  And processMainThreadEvents() is used when
  AppletHolder.setAppletVisible() is called with "wait" set to "true", but
  that currently never happens.)

Comment 7

[Simon Fraser [no longer active]]

So given that the event handing is necessary, which of the approaches in comment 2 sound possible, and can be made to work for both FF and Camino?

FWIW, I was not able to get Firefox to crash by doing the Command-W while popup is loading a Java applet trick, so I'm not sure if running this Cocoa event loop in a Carbon app is as risky as it is for a Cocoa app. But maybe our PLEvents could get processed (they are CFRunLoop based), which also might allow some dangerous re-entrancy.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #1 and following up comment #6)

> 1. Avoid having JEP do any event handling while waiting for awt/applet
>    loading.

As I've explained, this isn't doable.

> 3. Have JEP send out a notification before and after running one of these
>    event loops, which Camino can use to disable all its menu items. Not sure
>    how we'd fix Firefox to do something similar.

> 4. Have JEP add a method to MyNSApplication that Camino can call to tell if
>    such an event loop is running.

Either of these could (presumably) be easy for the JEP to do.

> 2. Have JEP only handle a subset of event types, or filter events to avoid
>    user events getting processed (again, security dialogs may make this
>    hard). Or maybe do something with event loop modes.

This would be a lot more work for me (and for the JEP), and might not be able
to handle all the problems cases (i.e. those aside from events that trigger a
window closing).  But it's worth a try.

I could either filter events directly (presumably at NSApplication sendEvent),
or I could "filter" their low-level results.  For example, I could hook
NSWindow close and never let that message through while the JEP is processing
events.  Instead I'd post a custom message to the NSApp queue, always leave it
in the queue when it's encountered while the JEP is processing events, but
then "process" it afterwards (from NSApplication sendEvent), by invoking the
"window close" command myself.

Comment 9

[Simon Fraser [no longer active]]

(In reply to comment #8)

> > 3. Have JEP send out a notification before and after running one of these
> >    event loops, which Camino can use to disable all its menu items. Not sure
> >    how we'd fix Firefox to do something similar.
> 
> > 4. Have JEP add a method to MyNSApplication that Camino can call to tell if
> >    such an event loop is running.
> 
> Either of these could (presumably) be easy for the JEP to do.

Both of course would require code changes in the application, which is unfortunate, but I'm willing to do at this stage. If we do this, we have to understand whether non-Cocoa apps will need anything.

> > 2. Have JEP only handle a subset of event types, or filter events to avoid
> >    user events getting processed (again, security dialogs may make this
> >    hard). Or maybe do something with event loop modes.
> 
> This would be a lot more work for me (and for the JEP), and might not be able
> to handle all the problems cases (i.e. those aside from events that trigger a
> window closing).  But it's worth a try.
> 
> I could either filter events directly (presumably at NSApplication sendEvent),
> or I could "filter" their low-level results.  For example, I could hook
> NSWindow close and never let that message through while the JEP is processing
> events.  Instead I'd post a custom message to the NSApp queue, always leave it
> in the queue when it's encountered while the JEP is processing events, but
> then "process" it afterwards (from NSApplication sendEvent), by invoking the
> "window close" command myself.

I'd prefer you didn't do this. It adds to the amount of evil hacking that the JEP does (and it already does enough), and the JEP can never know the set of events that might lead to bad things happening in the browser. The only reasonable filtering I can think of would be to just allow events to get processed for window that you know are owned entirely by Java (e.g. the security dialogs), but then you'd just have to throw the other events on the floor.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

> I'd prefer you didn't do this. It adds to the amount of evil hacking that
> the JEP does (and it already does enough)

The JEP is already a giant hack ... though a very clever and successful one.
What Apple has done with their JVM requires it.  There's no point in being
squeamish about the JEP getting just a little _more_ hackish :-)

> The only reasonable filtering I can think of would be to just allow events
> to get processed for window that you know are owned entirely by Java

This is worth thinking about for initWithParams (though I have my doubts it
will be possible even there).  It won't help for maybeCreateJavaVM, though.

Comment 11

[Simon Fraser [no longer active]]

Having the application listen for JEP notifications (which I have working now) is actually more tricky than I thought. The app will have to prevent any user actions which might cause the frames to be blown away, including close, reload, load a different url, close a tab etc. It's going to be very hard to catch all those scenarios.

Comment 12

[Boris Zbarsky [:bzbarsky]]

It'll also have to prevent JavaScript from running.  Or any existing close events being processed (consider a close event that's already in the event queue when the plugin instantiation event fires).

Comment 13

[Simon Fraser [no longer active]]

(In reply to comment #12)
> It'll also have to prevent JavaScript from running.  Or any existing close
> events being processed (consider a close event that's already in the event
> queue when the plugin instantiation event fires).

Yep. But at this point I'd be happy with a hack that catches 80% of the things that users can do to cause the crash (which probably means just preventing window/tab close in this time window). This is gonna be a branch-only hack.

Comment 14

[Simon Fraser [no longer active]]

Attachment: Re-entrant reflow via JEP

This stack shows that we can end up handling PLEvents during JEP's event loop, which means that we can re-enter reflow, and in this case I wasn't doing anything bad to the window, so this is going to happen on any page that is loading Java for the first time.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Yeah, not doing this from reflow is what bug 136927 is all about....

Comment 16

[Steven Michaud [:smichaud] (Retired)]

I've discovered something amazing -- at least I think its amazing.

As best I can tell I can now square the circle:  I can ensure that, while the
JEP is waiting for various stages of JVM or applet initialization (in
maybeCreateJavaVM and initWithParams), that the event loop(s) that the JVM
needs aren't blocked.  And at the same time I can ensure that Camino's (and
Firefox's) CFRunLoopSource (the one created in _md_CreateEventQueue() in
xpcom/threads/plevent.c) never gets called while the JEP is waiting.

Here's how I do it (I'm not at all sure how this works, but my tests have
shown that it _does_ work):

I make sure the NSApplication event queue (what NSApplication postEvent adds
events to) has at least one event in it -- my fake event will do.  Then I keep
calling NSApplication nextEventMatchingMask at short intervals, but without
ever dequeuing any event (so that the "next event" is always present, and
always the same).

That's all there is to it!

By playing around with gdb and NSLog, I discovered that
PL_ProcessPendingEvents() (called from _md_EventReceiverProc()) never gets
called until after the JEP has stopped waiting for an applet to load.

I need to do some more testing.  But (if all goes well) I hope to release a
new JEP "nightly" sometime within the next week.  Once it's available, you
guys can test with it and see what you think.

Comment 17

[Simon Fraser [no longer active]]

(In reply to comment #16)
> I've discovered something amazing -- at least I think its amazing.
> 
> I make sure the NSApplication event queue (what NSApplication postEvent adds
> events to) has at least one event in it -- my fake event will do.  Then I keep
> calling NSApplication nextEventMatchingMask at short intervals, but without
> ever dequeuing any event (so that the "next event" is always present, and
> always the same).

How do you not dequeue any event? By using an event mask of 0? I wonder if you could get the same effect by calling into NSRunLoop directly?

Comment 18

[Steven Michaud [:smichaud] (Retired)]

> How do you not dequeue any event?

I set nextEventMatchingMask's "dequeue" parameter to "false" :-)

(I set it's "mask" parameter to "NSAnyEventMask".)

Comment 19

[Simon Fraser [no longer active]]

Steven: any chance of a build to test with? If it's better, we'd like to get it into the Camino RC 1 build in the next few days.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

I hope to release a new JEP version (0.9.5+c) tomorrow that includes the
changes I described in comment #16.

I've done quite a bit of testing since then, and I've found that my solution
doesn't work on Mac OS X 10.2.8 (I need to keep using NSApp sendEvent on that
OS in initWithParms).  So (unless you guys do something) Camino will still be
vulnerable to bug 312062 on OS X 10.2.8.  But in other respects, everything I
said there has been borne out.

Comment 21

[Steven Michaud [:smichaud] (Retired)]

I've just released JEP 0.9.5+c.

http://javaplugin.sourceforge.net/

Please try it out and let me know the results of your tests.

By the way, I've found that my workaround also resolves issues that I had with
the bug 315111 URL on OS X 10.4.4, and which also seem to be caused by the
frame code being reentered:

https://bugzilla.mozilla.org/show_bug.cgi?id=315111#c13

Comment 22

[Steven Michaud [:smichaud] (Retired)]

This was probably fixed long ago.  And in any case the JEP is long gone.