This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 241533 - deadlock on IDE closing (OnStop ProgressUtils.showProgressDialogAndRun will block any AWT dialog)
Summary: deadlock on IDE closing (OnStop ProgressUtils.showProgressDialogAndRun will b...
Status: RESOLVED FIXED
Alias: None
Product: cnd
Classification: Unclassified
Component: -- Other -- (show other bugs)
Version: 8.0
Hardware: PC Solaris
: P2 normal (vote)
Assignee: Vladimir Voskresensky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-07 10:48 UTC by Maria Tishkova
Modified: 2014-02-16 06:51 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
thread dump (32.34 KB, text/plain)
2014-02-12 09:16 UTC, Vladimir Voskresensky
Details
dialog with "OK" below progress dlg (38.53 KB, image/png)
2014-02-12 09:22 UTC, Vladimir Voskresensky
Details
sample application (74.50 KB, application/x-tar)
2014-02-12 11:39 UTC, Vladimir Voskresensky
Details
patch proposal (1.12 KB, patch)
2014-02-12 13:50 UTC, Ondrej Vrabec
Details | Diff
patch in cnd??? (1.76 KB, patch)
2014-02-14 12:33 UTC, Ondrej Vrabec
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maria Tishkova 2014-02-07 10:48:52 UTC

    
Comment 1 Vladimir Voskresensky 2014-02-11 16:23:15 UTC
the problem is that ProgressUtils's dialog and DialogDisplayer do not know about each other. 
So, the following sequence cause deadlock:
- non EDT thread calls ProgressUtils.showProgressDialogAndRun(runnable)
- at the same time EDT thread should use DialogDisplayer 
=>
dlg created from DialogDisplayer is shown below progress dialog and no way to press buttons in it.
Looks like dlg does not use progress's windows as a parent, because it's not in NbPresenter.currentModalDialog field, because progress hasn't put it's dialog in this field
Comment 2 Ondrej Vrabec 2014-02-11 17:47:11 UTC
Attach a thread dump showing the DL. Thanks
Comment 3 Vladimir Voskresensky 2014-02-12 09:16:32 UTC
Created attachment 145076 [details]
thread dump
Comment 4 Vladimir Voskresensky 2014-02-12 09:21:28 UTC
"AWT-EventQueue-0" doing NbLifecycleManager.exit and calling ProjectOpenedHook$1.projectClosed
"On Start/Stop" calls RunOffEDTImpl.showProgressDialogAndRun, but runner can not run, because projectClosed displayed dialog which can not be closed by user. I will attach screenshot
Comment 5 Vladimir Voskresensky 2014-02-12 09:22:06 UTC
Created attachment 145077 [details]
dialog with "OK" below progress dlg
Comment 6 Ondrej Vrabec 2014-02-12 10:09:00 UTC
and how should it work? We discussed this with Standa and we agreed there's no way for us to fix this.
To be honest i am not following your locking mechanism. I assume the thread started by the progress dialog is:
"org.netbeans.modules.progress.ui.AbstractWindowRunner" - waiting on openedProjects.wait(). And who calls notify on that object? It gets notified after user clicks on OK in the underlying dialog, right?

But because:
1) the dialog expecting user input gets opened first
2) then you explicitely call ProgressUtils.showProgressAndRun which according to javadoc "blocks all user input and waits for the background thread to finish". So it does what it is expected to do.

So in our opinion you should fix this on your side and should not call showProgressDialogAndRun when you're currently showing a dialog requesting input.
Comment 7 Vladimir Voskresensky 2014-02-12 11:39:12 UTC
Created attachment 145083 [details]
sample application
Comment 8 Vladimir Voskresensky 2014-02-12 11:40:51 UTC
Ondrej, please, evaluate attached sample project
Comment 9 Vladimir Voskresensky 2014-02-12 11:42:28 UTC
in the attached sample project Progress is displayed and then user is asked for input, but input dlg is below progress dlg
Comment 10 Ondrej Vrabec 2014-02-12 13:50:10 UTC
Created attachment 145092 [details]
patch proposal
Comment 11 Ondrej Vrabec 2014-02-12 13:56:53 UTC
This patch may help you. However i will not commit it into the repo, i don't trust it fully, it may cause unexpected regressions during IDE startup. If it helps i suggest you push it to your branch you will build studio from.
Or, again, fix the deadlock in your code as you should. Once more ProgressUtils.showProgressDialogAndRun does what it's supposed to do: blocks all input until the background thread finishes - and because your background thread never finishes, the progress dialog will block until your notebook is out of power.
Comment 12 Vladimir Voskresensky 2014-02-12 14:47:45 UTC
Ondrej,

Thanks for the patch. Looks interesting. Additionally we can protect from start up regressions by "closed" flag and do it only during close phase.

As shown in my sample project. The situation is not specific to our code. It's just incorrect windows layout due to z-order.
Comment 13 Ondrej Vrabec 2014-02-12 15:37:44 UTC
(In reply to Vladimir Voskresensky from comment #12)
> As shown in my sample project. The situation is not specific to our code.
> It's just incorrect windows layout due to z-order.
Again, there is nothing such as incorrect z-order. The blocking progress dialog works as expected. It blocks literally *all* user input throughout NetBeans main window, all dialogs, popups etc.
Comment 14 Vladimir Voskresensky 2014-02-12 15:58:34 UTC
(In reply to Ondrej Vrabec from comment #13)
> (In reply to Vladimir Voskresensky from comment #12)
> > As shown in my sample project. The situation is not specific to our code.
> > It's just incorrect windows layout due to z-order.
> Again, there is nothing such as incorrect z-order. 
Do you mean, that "if" DialogDisplayer would have had progress's window as parent then I still will not be able to click buttons in DialogDisplayer's dialog?
Comment 15 Vladimir Voskresensky 2014-02-12 16:01:40 UTC
btw, javadoc of showProgressDialogAndRun claims that it blocks main window, not all other dialogs
Comment 16 Quality Engineering 2014-02-13 02:44:45 UTC
Integrated into 'main-silver', will be available in build *201402130001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/c3a4c1a53bbd
User: Vladimir Voskresensky <vv159170@netbeans.org>
Log: fixing #241533 - deadlock on IDE closing (OnStop ProgressUtils.showProgressDialogAndRun will block any AWT dialog)
- try cancellable runner
Comment 17 Vladimir Voskresensky 2014-02-13 12:07:33 UTC
patch doesn't help. 
What is the problem to remember progress's dialog and use it as the parent for DialogDisplayer and exception dialogs?
Comment 18 Ondrej Vrabec 2014-02-13 12:16:28 UTC
(In reply to Vladimir Voskresensky from comment #17)
> What is the problem to remember progress's dialog and use it as the parent
> for DialogDisplayer and exception dialogs?
The problem is that by the time the DialogDisplayer opens the modal dialog, the progress dialog is not even opened yet.
Comment 19 Vladimir Voskresensky 2014-02-13 12:17:16 UTC
What helps is:
        if (!mainWindow.isVisible()) {
            result.setModalityType(JDialog.ModalityType.MODELESS);
        }

Btw, if main window is not visible anymore, then modality makes not a lot of sense, isn't it?

if needed, we can try to protect ourselves to:
  if (!mainWindow.isVisible() && closingIDE) {
Comment 20 Ondrej Vrabec 2014-02-13 12:22:39 UTC
(In reply to Vladimir Voskresensky from comment #17)
> patch doesn't help. 
Well, it helped in the test app you provided. I was able to click on Yes/No in the other dialog.(In reply to Vladimir Voskresensky from comment #19)
> What helps is:
>         if (!mainWindow.isVisible()) {
>             result.setModalityType(JDialog.ModalityType.MODELESS);
>         }
> 
> Btw, if main window is not visible anymore, then modality makes not a lot of
> sense, isn't it?
Sure, you can set it to modeless if you want. But as good would be not to call showProgressAndRun during shutdown from your code.
Comment 21 Vladimir Voskresensky 2014-02-13 12:28:59 UTC
interesting observation. I've added action on toolbar doing:

    @Override
    public void actionPerformed(ActionEvent e) {
        ProgressUtils.showProgressDialogAndRun(new Runnable() {

            @Override
            public void run() {
                try {
                    Thread.sleep(2000);
                } catch (InterruptedException ex) {
                    Exceptions.printStackTrace(ex);
                }
                
                Object notify = DialogDisplayer.getDefault().notify(new NotifyDescriptor.Confirmation("Throw exception?", NotifyDescriptor.YES_NO_OPTION));
                if (notify == NotifyDescriptor.YES_OPTION) {
                    try {
                        Thread.sleep(2000);
                    } catch (InterruptedException ex) {
                        Exceptions.printStackTrace(ex);
                    }
                    Exceptions.printStackTrace(new IllegalStateException("Hello"));
                }
                
            }
        }, "Long task");
    }

And when main window is visible => 
- DialogDisplayer is visible on top of progress panel
- Exceptions is also displayed and clickable (progress panel is disappeared)
Comment 22 Jaroslav Tulach 2014-02-14 08:34:59 UTC
Vladimír wanted me to say something: As far as http://bits.netbeans.org/dev/javadoc/org-netbeans-api-progress/org/netbeans/api/progress/ProgressUtils.html#showProgressDialogAndRun(org.netbeans.api.progress.ProgressRunnable, java.lang.String, boolean) 
method goes, it currently says that it "blocks the main window", but as far as my knowledge of EDT goes, this in fact means to block all windows/dialogs that were shown so far. If this is part of the problem, then just update the javadoc to clarify that. In addition to that, if a new dialog is shown after, it should be operational.
Comment 23 Ondrej Vrabec 2014-02-14 08:44:57 UTC
(In reply to Vladimir Voskresensky from comment #21)
> interesting observation. I've added action on toolbar doing:
> 
>     @Override
>     public void actionPerformed(ActionEvent e) {
>         ProgressUtils.showProgressDialogAndRun(new Runnable() {
> 
>             @Override
>             public void run() {
>                 try {
>                     Thread.sleep(2000);
>                 } catch (InterruptedException ex) {
>                     Exceptions.printStackTrace(ex);
>                 }
>                 
>                 Object notify = DialogDisplayer.getDefault().notify(new
> NotifyDescriptor.Confirmation("Throw exception?",
> NotifyDescriptor.YES_NO_OPTION));
>                 if (notify == NotifyDescriptor.YES_OPTION) {
>                     try {
>                         Thread.sleep(2000);
>                     } catch (InterruptedException ex) {
>                         Exceptions.printStackTrace(ex);
>                     }
>                     Exceptions.printStackTrace(new
> IllegalStateException("Hello"));
>                 }
>                 
>             }
>         }, "Long task");
>     }
> 
> And when main window is visible => 
> - DialogDisplayer is visible on top of progress panel
> - Exceptions is also displayed and clickable (progress panel is disappeared)
If i'm getting this right then you're saying that when:
1) the blocking progress dialog is displayed (that's what showProgressDialogAndRun does
2) after that a dialog is displayed (DD.getDefault().notify)
=> then everything works?
Well that's what i've been talking about all the time. In your case your dialog asking the user is displayed earlier than the progress one.

I updated javadoc of ProgressUtils to say correctly what it does: blocks all displayed frames and dialogs: core-main #d506abe6e98a
Comment 24 Jaroslav Tulach 2014-02-14 09:01:42 UTC
(In reply to Vladimir Voskresensky from comment #1)
> the problem is that ProgressUtils's dialog and DialogDisplayer do not know
> about each other. 

Would it help if they knew about each other? The only cooperation between dialogs I am aware of is "isLeaf": http://bits.netbeans.org/dev/javadoc/org-openide-dialogs/org/openide/DialogDescriptor.html#isLeaf() but this helps only when dialogs are being closed - otherwise all dialogs follow Swing "nesting" rules.

> So, the following sequence cause deadlock:
> - non EDT thread calls ProgressUtils.showProgressDialogAndRun(runnable)
> - at the same time EDT thread should use DialogDisplayer 
> =>
> dlg created from DialogDisplayer is shown below progress dialog and no way
> to press buttons in it.

Yeah, that is something that can happen. As far as I know there is no support in the infrastructure to avoid that.

> Looks like dlg does not use progress's windows as a parent, because it's not
> in NbPresenter.currentModalDialog field, because progress hasn't put it's
> dialog in this field

I am not sure if this matters. Imho, it depends which dialog is shown sooner. If DialogDisplayer runs first and then ProgressUtils.showProgressDialogAndRun is executed - the first dialog will not be accessible.
Comment 25 Vladimir Voskresensky 2014-02-14 10:51:27 UTC
why can't we have:
        if (!mainWindow.isVisible()) {
            result.setModalityType(JDialog.ModalityType.MODELESS);
        }
?
I solves all problems
Comment 26 Ondrej Vrabec 2014-02-14 12:33:31 UTC
Created attachment 145197 [details]
patch in cnd???
Comment 27 Vladimir Voskresensky 2014-02-14 13:08:43 UTC
workaround on our side...
http://hg.netbeans.org/cnd-main/rev/3dc53f562f46
Comment 28 Vladimir Voskresensky 2014-02-14 13:09:37 UTC
(In reply to Ondrej Vrabec from comment #26)
> Created attachment 145197 [details]
> patch in cnd???
The whole purpose is to show dialog informing user, that IDE is really wasn't existed, jut windows was hidden and IDE have to finish flushing it's internal data
Comment 29 Quality Engineering 2014-02-15 05:12:18 UTC
Integrated into 'main-silver', will be available in build *201402150001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/d506abe6e98a
User: Ondrej Vrabec <ovrabec@netbeans.org>
Log: #241533 - deadlock on IDE closing (OnStop ProgressUtils.showProgressDialogAndRun will block any AWT dialog)
Updating javadoc to be in accordance with the implementation. The blocking progress dialog blocks all user input to all main window and displayed dialogs or frames.
Comment 30 Vladimir Voskresensky 2014-02-16 06:25:41 UTC
Related to deadlock on shutdown:
- may be we can introduce method for 'modeless' progress dlg?
Then in OnClose we can use it and no deadlocks can happen
Comment 31 Ondrej Vrabec 2014-02-16 06:51:42 UTC
(In reply to Vladimir Voskresensky from comment #30)
> Related to deadlock on shutdown:
> - may be we can introduce method for 'modeless' progress dlg?
> Then in OnClose we can use it and no deadlocks can happen
Yes, that makes sense.