2

A process which occurs in the background fires callbacks to ask various questions.

In this case the question is "is it okay to migrate your data?", so I have to ask the user. Since we have to do all Swing work on the EDT, this ends up looking like this (I only removed comments, reference to our own convenience methods and the parameters to allowMigration() - other than that, everything else is the same):

public class UserMigrationAcceptor implements MigrationAcceptor {
    private final Window ownerWindow;
    public UserMigrationAcceptor(Window ownerWindow) {
        this.ownerWindow = ownerWindow;
    }

    // called on background worker thread
    @Override
    public boolean allowMigration() {
        final AtomicBoolean result = new AtomicBoolean();
        try {
            SwingUtilities.invokeAndWait(new Runnable() {
                @Override
                public void run() {
                    result.set(askUser());
                }
            });
        } catch (InterruptedException e) {
            Thread.currentThread.interrupt();
            return false;
        } catch (InvocationTargetException e) {
            throw Throwables.propagate(e.getCause());
        }
        return result.get();
    }

    // called on EDT
    private boolean askUser() {
        int answer = JOptionPane.showConfirmDialog(ownerWindow, "...", "...",
                                                   JOptionPane.OK_CANCEL_OPTION);
        return answer == JOptionPane.OK_OPTION;
    }
}

What's happening is that in some situations, after confirming or cancelling the dialog which appears, Swing seems to get into the following state:

  • the JOptionPane is no longer visible
  • there is nothing pending on the event queue
  • the background thread is stuck inside #invokeAndWait, waiting for InvocationEvent#isDispatched() to return true.

Are we doing something wrong here, or am I looking at a bug in Swing/AWT?

The only other thing which might be worth noting is that this is the second level of modal dialogs. There is a modal dialog showing the progress of the operation and then this confirmation dialog has the progress dialog as its parent.

Update 1: Here's where the EDT is currently blocked:

java.lang.Thread.State: WAITING
      at sun.misc.Unsafe.park(Unsafe.java:-1)
      at java.util.concurrent.locks.LockSupport.park(LockSupport.java:186)
      at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:2043)
      at java.awt.EventQueue.getNextEvent(EventQueue.java:543)
      at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:211)
      at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:161)
      at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:154)
      at java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:182)
      at java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:221)
      at java.security.AccessController.doPrivileged(AccessController.java:-1)
      at java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:219)
      at java.awt.Dialog.show(Dialog.java:1082)
      at java.awt.Component.show(Component.java:1651)
      at java.awt.Component.setVisible(Component.java:1603)
      at java.awt.Window.setVisible(Window.java:1014)
      at java.awt.Dialog.setVisible(Dialog.java:1005)
      at com.acme.swing.progress.JProgressDialog$StateChangeListener$1.run(JProgressDialog.java:200)
      at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:251)
      at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:733)
      at java.awt.EventQueue.access$200(EventQueue.java:103)
      at java.awt.EventQueue$3.run(EventQueue.java:694)
      at java.awt.EventQueue$3.run(EventQueue.java:692)
      at java.security.AccessController.doPrivileged(AccessController.java:-1)
      at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
      at java.awt.EventQueue.dispatchEvent(EventQueue.java:703)
      at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:242)
      at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:161)
      at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:154)
      at java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:182)
      at java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:221)
      at java.security.AccessController.doPrivileged(AccessController.java:-1)
      at java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:219)
      at java.awt.Dialog.show(Dialog.java:1082)
      at javax.swing.JOptionPane.showOptionDialog(JOptionPane.java:870)

What's odd here is that the showOptionDialog() at the bottom is the migration prompt, but the Dialog#setVisible further up is the progress dialog. In other words, somehow sometimes the child dialog appears before the parent, and perhaps this is what is breaking Swing.

Update 2:

And indeed, I can make this happen in a test program without using any of our own code. Although the dialog positioning is different in the test program, it hangs in exactly the same way, only more reproducibly. gist

Trejkaz
  • 10,264
  • 6
  • 52
  • 106
  • For better help sooner, post an [SSCCE](http://sscce.org/). As an aside, I suspect it is freezing because either a) the code is blocking the EDT, or b) `invokeAndWait` is used instead of `invokeLater`. OTOH I will know more when you post an SSCCE. – Andrew Thompson Jul 22 '13 at 06:59
  • Difficult, but will try. At the moment, the EDT is waiting to process a new event but no events are coming. As far as I can tell, InvocationEvent.dispatch() is being called but the dispatched = true line is not being executed the second time around. Also invokeLater is impossible to use, because the caller is expecting me to return a value. – Trejkaz Jul 22 '13 at 07:13
  • 1
    The basic example seems to work for me...I was able to cycle it over 100 times with out isse – MadProgrammer Jul 22 '13 at 07:22
  • When this was reported in production, I was also unable to reproduce it in the same way the issue was reported. They said it occurred on OKing the dialog, I find that it only occurs on cancelling it for me. Also it only occurs on the second and subsequent invocations, so I think it's some kind of race condition. Likewise, my trivial sample program isn't simulating what really happens well enough, so it isn't happening there either. I don't have any idea of the sort of nanosecond timing I need to force it to happen in 100% of cases. :( – Trejkaz Jul 22 '13 at 07:29
  • the example works fine here - any specifics of OS/LAF/jdk when it blows? (not very helpful, I know, just saying ;-) – kleopatra Jul 23 '13 at 09:17
  • Windows 8, Java 7, L&F seems to be irrelevant because it occurs on all the ones I have available. – Trejkaz Jul 24 '13 at 23:50
  • Mac OS X and Java 7 with Quaqua L&F also causes it, so I don't think this is necessarily a platform-specific issue. I'll probably post another question about the recommended way to go about unwinding this sort of stuff. :( – Trejkaz Oct 16 '13 at 22:46

3 Answers3

2

I just had this same issue in my own code.

Your call to JOptionPane.showOptionDialog() never returns, because while its event dispatch loop is sitting there waiting for user input, a timer (or something else) has fired and caused another modal dialog to install its own event loop. In your stack, the culprit is JProgressDialog$StateChangeListener$1.run(), which you can then see launch its own event dispatch loop.

Until your JProgressDialog is closed, it won't exit its loop, and the earlier call to JOptionPane.showOptionDialog() will never return.

This might not be obvious if the dialog parents seem to imply a hierarchy that isn't honored by the event queue.

Two solutions might be to either avoid a modal progress dialog, or show the progress dialog immediately. Launching a modal dialog off an event is only going to be a good idea if the rest of the event thread is happy to sit back and wait for it to be closed.

BenM
  • 86
  • 3
  • Showing the progress dialog sooner did fix the issue actually (though I had figured it out myself first and never saw the notification about this answer). I'll still mark this as the answer for this particular issue. – Trejkaz Mar 21 '14 at 02:01
1
  • invokeAndWait must be called out of EDT,

  • carefully with invokeAndWait, because can freeze whole Swing GUI, locked by exceptions from RepaintManager, not in all cases only GUI is created, relayout, refreshed some of methods, when repaint() is called from nested methods

  • for invokeAndWait is required to test if (EventQueue.isDispatchThread()) { / if (SwingUtilities.isEventDispatchThread()) {

  • on true from isDispatchThread() you can to result.set(askUser());) without any side effects, output is done on EDT, but is about good practicies to wrap inside invokeLater

  • I seen some usages of invokeAndWait but only on application start_up, use invokeLater() instead

mKorbel
  • 108,320
  • 17
  • 126
  • 296
  • I think I will try using invokeLater, but it seems I still have to block the caller until it actually executes, so essentially I will be reimplementing invokeAndWait. i.e., if doing that works, then invokeAndWait almost certainly has a bug. :( – Trejkaz Jul 22 '13 at 07:36
  • true is that I never see real usage of invokeAndWait in Java >= 6, [this is simple example about logics how EDT works](http://stackoverflow.com/a/11204639/714968), if are events are flushed (bunch of events implemented in API) then EDT returns false (there is delayed on 30seconds), simulation, [how is EDT implemented in API(ignore code inside if - else)](http://stackoverflow.com/a/6149571/714968), your code would need this sceleton as notifier to EDT – mKorbel Jul 22 '13 at 07:47
0

With this kind of chaos, the first thing to suspect is that, somewhere, a Swing method is being called off the EventQueue (EDT). Swing often works very well with multiple threads, but then every once in a while this happens. Unfortunately, I know of no other way to to fix the problem than checking each and every swing method call and making sure it's on the EDT. (Note, there are one or two Swing methods that can run on other threads, such as repaint, but check the source code and Javadoc for each one to be sure.)

RalphChapin
  • 2,874
  • 13
  • 17
  • We actually have a custom repaint manager in place such that if anything ultimately repaints off the EDT, an exception is thrown. That isn't being triggered, so if any Swing stuff is being called off the EDT, I at least know that it isn't repaint()... but it could be anything else which might not trigger a repaint. :( – Trejkaz Jul 22 '13 at 23:03
  • I thought `repaint` was safe, but I just checked the source and it isn't and, I'm now sure, never was. Good luck with this. You have to spot _every_ Swing method call, then determine if that code can run off the EDT. I've thought that every Swing method _ought_ to do its own check and throw and exception if it's _not_ on the EDT, but that that would probably slow it down too much. – RalphChapin Jul 23 '13 at 13:17
  • _I just checked the source and it isn't_ hmm ... how do you reach that conclusion? AFAICS, it posts PaintEvents to the EventQueue which is running on the EventDispatchThread. – kleopatra Jul 23 '13 at 15:24
  • @kleopatra: That's what I thought when I answered the question. I checked repaint in JComponent. At a brief glance, it (and RepaintManager) are not thread safe. More importantly, the documentation says nothing about being thread safe. repaint for awt.Component looks a lot safer, _and_ calls postEvent directly. At any rate, I shouldn't be talking about repaint until I've done a lot more (post 1.4) research. – RalphChapin Jul 23 '13 at 15:45
  • repaint() is *documented* as safe, but what we find is that 99.999% of calls to repaint() are side-effects of fiddling with models on the wrong thread, so blocking it from other threads has caught quite a bit of mischief. In the few components which do it without us having a choice (e.g. anything where you're painting an animated GIF) we put in a client property to say that we know the component is badly behaved (and of course, all of this only runs in development.) – Trejkaz Jul 24 '13 at 23:55
  • @Trejkaz: Thanks. I once knew all about repaint(). I'll need to know all about it in the future, and I find my current ignorance rather annoying/embarrassing/awkward. – RalphChapin Jul 25 '13 at 01:22