Closed Bug 860490 Opened 12 years ago Closed 11 years ago

Firefox stops repainting when an embedded Java applet launches a dialog

Categories

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

Product:
Core Graveyard
Component:
Plug-ins
Version:
20 Branch
Platform:
x86_64
Windows 7
Type:
defect

Tracking

(firefox22 wontfix, firefox23 wontfix, firefox24 verified, firefox25 verified, firefox-esr17 unaffected, b2g18 unaffected)

Status:
VERIFIED FIXED
Milestone:
mozilla25
Tracking Flags:
Tracking Status
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 --- verified
firefox25 --- verified
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: vpotok, Assigned: johns)

References

Details

(Keywords: regression)

Attachments

(1 file)

Mark _pluginthreadasynccall calls as safe to re-enter gecko
784 bytes, patch
benjamin
: review+
roc
: review+
bajaj
: approval-mozilla-aurora+
johns
: checkin+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130307023931

Steps to reproduce:

Launch a webpage that contains an embedded Java applet.  Use the Java applet to open a Java dialog.

Sample steps to reproduce:
1. Go Here: http://jupload.sourceforge.net/advanced_js_demo.html
2. Make sure Java applet is fully loaded
3. Click on the Browse button within the Java applet to launch a Java dialog. 
4. Attempt to open a new Firefox tab and navigate to another page.


Actual results:

Once the Java dialog is displayed, all currently opened Firefox windows and tabs stop repainting themselves.  Any change made within a Firefox window or tab will not be reflected unless focus is shifted to another application and then shifted back to Firefox.

Reproducible starting with Firefox 18 (and onward) on Windows 7/8 regardless of JRE version.


Expected results:

User should be able to freely switch between Firefox tabs and open more Firefox windows.  Firefox should not stop repainting itself in this scenario.
It's not a recent issue, it's reproducible in FF8. And likely a dupe.
Please use these steps to reproduce instead:
1. Go Here:  http://jupload.sourceforge.net/advanced_js_demo.html
2. Make sure Java applet is fully loaded
3. Click on the Browse button within the Java applet to launch a Java dialog. 
4. Open a new Firefox window (open a new instance of Firefox in Windows) and attempt navigate to another page.



(In reply to V from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101
> Firefox/19.0
Build ID: 20130307023931

Steps to reproduce:

Launch a webpage
> that contains an embedded Java applet.  Use the Java applet to open a Java
> dialog.

Sample steps to reproduce:
1. Go Here:
> http://jupload.sourceforge.net/advanced_js_demo.html
2. Make sure Java
> applet is fully loaded
3. Click on the Browse button within the Java applet
> to launch a Java dialog. 
4. Attempt to open a new Firefox tab and navigate
> to another page.


Actual results:

Once the Java dialog is displayed, all
> currently opened Firefox windows and tabs stop repainting themselves.  Any
> change made within a Firefox window or tab will not be reflected unless
> focus is shifted to another application and then shifted back to Firefox.
> Reproducible starting with Firefox 18 (and onward) on Windows 7/8 regardless
> of JRE version.


Expected results:

User should be able to freely switch
> between Firefox tabs and open more Firefox windows.  Firefox should not stop
> repainting itself in this scenario.
Dupe of bug 860052, bug 850191 or bug 832963 ?
Any update on this?  This seems like a major defect.
Comment 2 sounds like bug 860490 to me, could we confirm this?
(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> Comment 2 sounds like bug 860490 to me, could we confirm this?

it would be nicer to close it as a duplicate per comment 3
A dupe of which bug?
Any update on this one?  I am able to reproduce in Firefox 21.  All instances of Firefox windows and tabs stop redrawing themselves when a Java dialog is opened. This seems like a major defect!

Steps to reproduce:

1. Go Here:  http://jupload.sourceforge.net/advanced_js_demo.html
2. Make sure Java applet is fully loaded
3. Click on the Browse button within the Java applet to launch a Java dialog. 
4. Open a new Firefox window or tab (Firefox fails to repaint itself)
Thanks V!
This is reproducible on FF 4, so it doesn't seem to be a regression.
Setting to NEW.
Sure Paul, this Firefox bug is actually greatly impacting a major enterprise application.  Would it be possible to get a status update on this bug?  Has anyone looked into this yet/are there any potential fixes in the pipeline?
Unfortunately, I think there aren't too many updates on this. Someone from development needs to assign on it first and start working.
This sounds like a bug 785348 regression, similar to the aforementioned bugs, but that would not explain why others are able to reproduce this on earlier versions. It's possible that we're seeing two different bugs here.

V, can you verify that this does not reproduce for you on FF17 or earlier? If you could use the mozregression tool to find the first nightly where the issue occurs for you it would help in narrowing this down:
http://mozilla.github.io/mozregression/
John, I had issues with mozregression, but was able to manually confirm that this issue DOES NOT occur on Firefox Setup 17.0b6 or earlier.  It DOES occur on FF 18.0 and above. 

Could somebody please look into what has changed?
(In reply to V from comment #13)
> John, I had issues with mozregression, but was able to manually confirm that
> this issue DOES NOT occur on Firefox Setup 17.0b6 or earlier.  It DOES occur
> on FF 18.0 and above. 
> 
> Could somebody please look into what has changed?

I don't think so, I tried with FF8 and when the Java modal dialog is open, the user cannot interact with the Firefox UI (pressing buttons, opening new tab etc), like if the main thread was blocked.
Loic, let me clarify.  

In Firefox 17b6 and earlier you can have a Java modal dialog open in one Firefox window and still carry on doing your business in another unrelated Firefox window.

As of Firefox 18, having a Java modal dialog open in one Firefox window directly prevents all other Firefox windows from repainting.

Hope that helps...
Note: I am referring to Firefox "windows" not "tabs" in my above comment.
Ok, because your STR weren't very clear, sometimes it was about a tab, sometimes about a window.

So, should I open 2 Firefox windows before opening a Java modal dialog in one of them?
In Firefox 17.0b6
Window 1: Open a Java modal dialog
Window 2. Perform any action, such as typing a URL into the address bar.  NOTE: This window can be opened at any time (before, or after opening the Java modal dialog in Window 1) and it will function as expected.

In Firefox 18.0
Window 1: Open a Java modal dialog
Window 2. Perform any action, such as typing a URL into the address bar.  NOTE: This window can be opened at any time (before, or after opening the Java modal dialog in Window 1) and it will NOT repaint whilst the Java modal dialog is opened in Window 1.  If you close the Java modal dialog in Window 1, then Window 2 will start repainting again.
(In reply to V from comment #13)
> John, I had issues with mozregression, but was able to manually confirm that
> this issue DOES NOT occur on Firefox Setup 17.0b6 or earlier.  It DOES occur
> on FF 18.0 and above. 
> 
> Could somebody please look into what has changed?

Bug 785348 landed, as mentioned above. If it works on 17 and breaks on 18, odds are great that this is the culprit.

@roc - This bug is a slight variant on bug 860052 / bug 850191 / bug 832963, wherein java captures the event loop when doing modal things, and bug 785348 makes us cease painting. This is pretty bad when it seems to be a common java behavior -- is there someone that can look into this on the graphics side, or are we effectively WONTFIXing?
Ok, I'm able to repro it.

STR:
1) Open 2 Firefox windows (Ctrl+N)
2) In the 1st window, go to http://jupload.sourceforge.net/advanced_js_demo.html 
3) When the Java applet is loaded, click on "Browsing" to open a modal dialog
4) Switch to the 2nd window

Result: almost impossible to interact with the UI of the 2nd window, repainting is frozen.

Regression range:
good=2012-12-10
bad=2012-12-11
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=725eb8792d27&tochange=4dfe323a663d

Robert O'Callahan — Bug 785348. Part 1: Track when we've called into plugin code. While we're in plugin code, never run the refresh driver. r=mats
It's important to know what the stack looks like in these bugs when Java spins its nested event loop. With some stacks, we know it would be safe for Firefox to do arbitrary things (e.g. painting) in the nested event loop, and we can whitelist the associated plugin entry points like we did for event handlers in https://hg.mozilla.org/releases/mozilla-beta/rev/2c851ecf2db5. For other stacks, allowing Firefox to paint (or potentially do other things) causes bad reentrancy leading to crashes like bug 785348.

The description in comment #2 suggests that the nested event loop is triggered by an input event, but if so it would have been fixed by bug 829557. So the dialog must be triggered indirectly somehow.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #21)
> It's important to know what the stack looks like in these bugs when Java
> spins its nested event loop. With some stacks, we know it would be safe for
> Firefox to do arbitrary things (e.g. painting) in the nested event loop, and
> we can whitelist the associated plugin entry points like we did for event
> handlers in https://hg.mozilla.org/releases/mozilla-beta/rev/2c851ecf2db5.
> For other stacks, allowing Firefox to paint (or potentially do other things)
> causes bad reentrancy leading to crashes like bug 785348.
> 
> The description in comment #2 suggests that the nested event loop is
> triggered by an input event, but if so it would have been fixed by bug
> 829557. So the dialog must be triggered indirectly somehow.

I (painfully) debugged this a bit - it looks like java passes input events to its thread, which then uses NPN_PluginThreadAsyncCall to put an event on the main loop, where it captures it from a top level nsPluginThreadRunnable::Run. There are other ways to get into plugins from non-input events though, such as simple timers that run JS that calls into java. Would it make sense to invert our method here and explicitly mark when we're in *dangerous* pathways (e.g. Destroy)? Do we know of other issues besides plugins re-entering during document teardown?
Whitelisting known good paths is generally safer than blacklisting known bad paths.

nsPluginThreadRunnable::Run() should definitely be whitelisted. So should _evaluate, _invoke and _invokeDefault, since any access to the plugin by script must be safe, since script itself can trigger a nested event loop.
Can we plan for this bug to be addressed in a future Firefox release?  If so, any idea of which release might be targetted?
Note that when Java moves back out of process on Windows (bug 874176) then nsPluginThreadRunnable will be entirely in the plugin process and won't affect the rest of this.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23)
> Whitelisting known good paths is generally safer than blacklisting known bad
> paths.
> 
> nsPluginThreadRunnable::Run() should definitely be whitelisted. So should
> _evaluate, _invoke and _invokeDefault, since any access to the plugin by
> script must be safe, since script itself can trigger a nested event loop.

Let's try this, and handle the cases in the similar bugs there if they persist.
(In reply to V from comment #24)
> Can we plan for this bug to be addressed in a future Firefox release?  If
> so, any idea of which release might be targetted?

This is fundamentally a bad behavior by Java (nested event loops), but we're going to try to expand our workarounds to accommodate it. If that resolves the issue without further regressions, it may be uplifted to firefox 24, which is the base for the next ESR release IIRC
This seems to fix the testcase in comment 0. (_invoke and other script calls are not tracked as unsafe calls to begin with)

https://tbpl.mozilla.org/?tree=Try&rev=d4792f0679c9
Comment on attachment 782726 [details] [diff] [review]
Mark _pluginthreadasynccall calls as safe to re-enter gecko

What happens if this runs inside a nested event loop where we claim it's not ok to reenter gecko?


* ?
* nsPluginThreadRunnable::Run
* Java spins nested loop
* content -> NPP_Shutdown
* main
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #29)
> Comment on attachment 782726 [details] [diff] [review]
> Mark _pluginthreadasynccall calls as safe to re-enter gecko
> 
> What happens if this runs inside a nested event loop where we claim it's not
> ok to reenter gecko?
> 
> 
> * ?
> * nsPluginThreadRunnable::Run
> * Java spins nested loop
> * content -> NPP_Shutdown
> * main

As long as there's any unsafe to reenter call on the stack InPluginCallUnsafeForReentry will return true: http://dxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsNPAPIPluginInstance.h#l293
(In reply to V from comment #16)
> Note: I am referring to Firefox "windows" not "tabs" in my above comment.
Should be able to open other tabs too, like Google Chrome. Perhaps this could be also fixed here.
Comment on attachment 782726 [details] [diff] [review]
Mark _pluginthreadasynccall calls as safe to re-enter gecko

https://hg.mozilla.org/integration/mozilla-inbound/rev/e70411983e39
https://hg.mozilla.org/mozilla-central/rev/e70411983e39
Closed: 11 years ago
I guess we should uplift this.
It would be great if this will land earlier.
Comment on attachment 782726 [details] [diff] [review]
Mark _pluginthreadasynccall calls as safe to re-enter gecko

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 785348 

User impact if declined: Many Java applets that make use of modal dialogs cause the browser to cease repainting, appearing frozen. This will probably be especially painful in the next ESR release where java is used more heavily. Suspected to fix this bug as well as bug 832963, bug 850191, and bug 860052.

Testing completed (on m-c, etc.): On m-c

Risk to taking this patch (and alternatives if risky): Low, marks a pathway heavily (and probably exclusively) used by java as safe to re-enter. Said pathway is always a top level event so is necessarily safe to reenter.

String or IDL/UUID changes made by this patch:
None
roc, do you think this is low-risk enough for landing on beta before the uplift?
I would have thought beta is already completely locked down for the release. So, no.
nightly 25.0a1 (2013-07-31), win 7 x64
After "3. Click on the Browse button within the Java applet to launch a Java dialog" I'm still not able to enter anything in the location bar or to open new tabs.
The only thing fixed is that the other FF windows are no longer frozen.
Is this ok ?
(In reply to Paul Silaghi [QA] from comment #39)
> nightly 25.0a1 (2013-07-31), win 7 x64
> After "3. Click on the Browse button within the Java applet to launch a Java
> dialog" I'm still not able to enter anything in the location bar or to open
> new tabs.
> The only thing fixed is that the other FF windows are no longer frozen.
> Is this ok ?

Java is capturing the event loop and stealing input events to the window -- it's trying to display a modal dialog, but not doing it entirely sanely. Unfortunately this is just something plugins can do if they choose to :(
(In reply to John Schoenick [:johns] from comment #40)
> Unfortunately this is just something plugins can do if they choose to :(
See Chrome's behavior.
(In reply to Paul Silaghi [QA] from comment #41)
> (In reply to John Schoenick [:johns] from comment #40)
> > Unfortunately this is just something plugins can do if they choose to :(
> See Chrome's behavior.

Right, that would be https://bugzilla.mozilla.org/show_bug.cgi?id=e10s
(In reply to John Schoenick [:johns] from comment #42)
> Right, that would be https://bugzilla.mozilla.org/show_bug.cgi?id=e10s

Actually bug 874167 might be enough to prevent this as well
Ok, thanks John.
Verified based on comments 39-43.
Verified fixed FF 24b1 Win 7
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: