Last Comment Bug 671820 - A file picker and an alert can be shown at the same time, making the window unresponsive
: A file picker and an alert can be shown at the same time, making the window u...
: hang, pp, testcase
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: All Linux
: -- normal with 2 votes (vote)
: mozilla17
Assigned To: Karl Tomlinson (:karlt)
Depends on: 731307
Blocks: 59314
  Show dependency treegraph
Reported: 2011-07-15 03:28 PDT by Ben Bucksch (:BenB)
Modified: 2016-03-05 01:11 PST (History)
11 users (show)
karlt: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Screenshot of deadlock: both File Upload dialog and alert() open at the same time (236.66 KB, image/png)
2011-07-15 03:32 PDT, Ben Bucksch (:BenB)
no flags Details
testcase (WARNING: will block the entire window!) (275 bytes, text/html)
2011-07-18 16:16 PDT, Mounir Lamouri (:mounir)
no flags Details
implement async nsIFilePicker::Open and make sync Show close window on response (10.74 KB, patch)
2012-08-21 20:23 PDT, Karl Tomlinson (:karlt)
no flags Details | Diff | Splinter Review

Description Ben Bucksch (:BenB) 2011-07-15 03:28:32 PDT
I am currently in a situation where I cannot close the native file picker dialog that opens for a <input type="file"> element on a webpage. I can use the dialog just fine, it's working, just the "Cancel" and "Open" buttons have no effect. I can still use other Firefox windows, luckily, but I cannot access the Firefox window that opened the dialog, I cannot close that Firefox window either. I am stuck.

I had pressed the "Open" button of the dialog before. The webpage, in the background, was opening an alert() (apparently) that shows an error message from the web app. This might be what is causing the deadlock.

I see nothing relevant on the error console.
The web app is the admin frontend of a phone system (gigaset IP).
Comment 1 Ben Bucksch (:BenB) 2011-07-15 03:32:11 PDT
Created attachment 546124 [details]
Screenshot of deadlock: both File Upload dialog and alert() open at the same time
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-07-15 06:35:57 PDT
This has nothing to do with form submission; it sounds like an OS-specific event delivery issue...
Comment 3 Ben Bucksch (:BenB) 2011-07-15 07:32:40 PDT
In case you need me to check something, I still have the window open, but I'll have to close it some time.
Comment 4 Mounir Lamouri (:mounir) 2011-07-18 16:16:41 PDT
Created attachment 546663 [details]
testcase (WARNING: will block the entire window!)

I managed to create a testcase for it. Would be nice to have someone reproducing that on Windows or MacOS X to check if that's really platform specific.
Comment 5 Mounir Lamouri (:mounir) 2011-07-18 17:47:31 PDT
Actually, it breaks MacOS X and GTK but not Windows... and the way it breaks on MacOS X is different than GTK: the alert is never shown and the application is hanging when you click on OK or CANCEL.
Comment 6 Alice0775 White 2011-07-18 22:58:03 PDT
If I set prompts.tab_modal.enabled to false, Browser does not hang on Nightly 8.0a1 of Windows and Linux.(not testing on MacOS)
Comment 7 Pierre Réveillon 2012-04-16 23:38:47 PDT
Isn't it the same bug as #476541?
Comment 8 Pierre Réveillon 2012-04-16 23:41:21 PDT
Soory for the missing link, it's bug 476541.
Comment 9 Mounir Lamouri (:mounir) 2012-04-17 00:45:49 PDT
(In reply to Pierre Réveillon from comment #7)
> Isn't it the same bug as #476541?

On Mac yes. But it seems like it's reproducible using the GTK widget.
We could make this GTK though.
Comment 10 Ben Bucksch (:BenB) 2012-04-17 04:56:45 PDT
Per comment 2, back to GTK
Comment 11 Karl Tomlinson (:karlt) 2012-04-17 20:48:00 PDT
Nested event loops are evil, and multiple nested event loops are more evil
(if there is such a thing).

The bug is in the nsIFilePicker::show interface because the "show" method
can't return until it knows what the user selects.  That means that either the
whole application needs pause while waiting for the response or the "show"
implementation needs to run a nested event loop. 

Further gtk_dialog_run doesn't hide the modal window.  It is not closed until
gtk_dialog_run returns which will not happen until the nested event loop

The tab-modal alert is implemented with another nested event loop.
That means that the gtk_dialog_run nested event loop can't return until the
tab-modal alert is dismissed.  The modal dialog prevents the tab-modal alert
from being dismissed.

We could adjust the GTK nsIFilePicker implementation to hide the dialog when
the buttons are pressed (instead of waiting for the event loop to exit), but
there would still be a bug:  The file would not be selected until the tab-modal
alert has been dismissed.

Bug 729720 comment 4 also wanted to make the filepicker interface async.
Comment 12 Karl Tomlinson (:karlt) 2012-04-17 21:06:38 PDT
A similar bug exists on Win7, although there is a race condition.
If the file picker opens before the alert, then the file picker can be dismissed and then the alert.

However, sometimes the file picker does not open but the window with the alert becomes unresponsive.
Comment 13 Karl Tomlinson (:karlt) 2012-04-18 16:35:43 PDT
Steven's work in bug 729720 comment 26 would go a long way in helping here.
Comment 14 Ben Bucksch (:BenB) 2012-04-18 16:42:55 PDT
Thanks for the reference, but I saw the bug on GTK.
Comment 15 Karl Tomlinson (:karlt) 2012-04-18 16:54:09 PDT
Part of Steven's patchset is cross platform to provide an async show() method.
Comment 16 Mounir Lamouri (:mounir) 2012-04-19 02:08:14 PDT
I think this bug should depend on bug 731307 instead.
Comment 17 Gábor Stefanik 2012-06-12 14:29:46 PDT
Isn't this security-sensitive?
Comment 18 Mounir Lamouri (:mounir) 2012-06-19 12:48:56 PDT
Those bugs where "sec:dos" before. How should they be marked now Jesse?
Comment 19 Jesse Ruderman 2012-06-19 13:42:44 PDT
Keep using [sg:dos] for now. We might add a csec-dos keyword eventually.
Comment 20 Mounir Lamouri (:mounir) 2012-08-20 13:04:39 PDT
This should have been fixed by bug 731307. Might be nice if that could be checked by QA using the latest Nightly.
Comment 21 Karl Tomlinson (:karlt) 2012-08-21 17:18:09 PDT
Bug 731307 added suitable API, and moves the nested event loop to run off an event, but, to fix the bug, we still need to not wait for the nested event loop to return.
Comment 22 Karl Tomlinson (:karlt) 2012-08-21 20:23:33 PDT
Created attachment 654062 [details] [diff] [review]
implement async nsIFilePicker::Open and make sync Show close window on response

Even the sync Show() variant doesn't demonstrate this lock-up any longer due to closing the window before waiting for the stack to unwind, but that method still has other bugs related to not unwinding the stack.  

gtk_window_set_destroy_with_parent is used to release the nsIFilePickerShownCallback (for the async variant) before XPCOM shutdown, when quitting from another window while the file dialog is still open.  (That is the only reason the "destroy" handler is required.)  The file picker would be of limited use without a parent anyway.
Comment 23 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-21 21:02:20 PDT
Comment on attachment 654062 [details] [diff] [review]
implement async nsIFilePicker::Open and make sync Show close window on response

Review of attachment 654062 [details] [diff] [review]:


::: widget/gtk2/nsFilePicker.cpp
@@ +377,5 @@
> +  nsresult rv = Open(nullptr);
> +  if (NS_FAILED(rv))
> +    return rv;
> +
> +  while(mRunning) {

space before (

::: widget/xpwidgets/nsBaseFilePicker.cpp
@@ +101,5 @@
>    nsCOMPtr<nsIRunnable> filePickerEvent =
>      new AsyncShowFilePicker(this, aCallback);
>    return NS_DispatchToMainThread(filePickerEvent);
>  }
> +#endif

Do we really need these #ifdefs? Seems to me we don't.
Comment 24 Karl Tomlinson (:karlt) 2012-08-21 23:57:19 PDT
The ifdefs were to help clarify which code in the override hierarchy actually gets used, but dead code is generally left in the build, so followed style there.

There was a single try build here:
It is hard to test this as it is difficult to send events to native dialogs.
Comment 25 Ed Morley [:emorley] 2012-08-23 03:52:21 PDT
Comment 26 Merih Akar 2012-11-21 12:57:20 PST
The bug appears to be fixed in FF17, however I am currently using FF17 and the test page above still blocks the entire window. Am I missing something?

Note You need to log in before you can comment on or make changes to this bug.