Unresponsive threading involving Swing and AWT-EventQueue

Question

I have an application that is unresponsive and seems to be in a deadlock or something like a deadlock. See the two threads below. Notice that the My-Thread@101c thread blocks AWT-EventQueue-0@301. However, My-Thread has just called java.awt.EventQueue.invokeAndWait(). So AWT-EventQueue-0 blocks My-Thread (I believe).

My-Thread@101c, priority=5, in group 'main', status: 'WAIT'
     blocks AWT-EventQueue-0@301
      at java.lang.Object.wait(Object.java:-1)
      at java.lang.Object.wait(Object.java:485)
      at java.awt.EventQueue.invokeAndWait(Unknown Source:-1)
      at javax.swing.SwingUtilities.invokeAndWait(Unknown Source:-1)
      at com.acme.ui.ViewBuilder.renderOnEDT(ViewBuilder.java:157)
        .
        .
        .
      at com.acme.util.Job.run(Job.java:425)
      at java.lang.Thread.run(Unknown Source:-1)

AWT-EventQueue-0@301, priority=6, in group 'main', status: 'MONITOR'
     waiting for My-Thread@101c
      at com.acme.persistence.TransactionalSystemImpl.executeImpl(TransactionalSystemImpl.java:134)
        .
        .
        .
      at com.acme.ui.components.MyTextAreaComponent$MyDocumentListener.insertUpdate(MyTextAreaComponent.java:916)
      at javax.swing.text.AbstractDocument.fireInsertUpdate(Unknown Source:-1)
      at javax.swing.text.AbstractDocument.handleInsertString(Unknown Source:-1)
      at javax.swing.text.AbstractDocument$DefaultFilterBypass.replace(Unknown Source:-1)
      at javax.swing.text.DocumentFilter.replace(Unknown Source:-1)
      at com.acme.ui.components.FilteredDocument$InputDocumentFilter.replace(FilteredDocument.java:204)
      at javax.swing.text.AbstractDocument.replace(Unknown Source:-1)
      at javax.swing.text.JTextComponent.replaceSelection(Unknown Source:-1)
      at javax.swing.text.DefaultEditorKit$DefaultKeyTypedAction.actionPerformed(Unknown Source:-1)
      at javax.swing.SwingUtilities.notifyAction(Unknown Source:-1)
      at javax.swing.JComponent.processKeyBinding(Unknown Source:-1)
      at javax.swing.JComponent.processKeyBindings(Unknown Source:-1)
      at javax.swing.JComponent.processKeyEvent(Unknown Source:-1)
      at java.awt.Component.processEvent(Unknown Source:-1)
      at java.awt.Container.processEvent(Unknown Source:-1)
      at java.awt.Component.dispatchEventImpl(Unknown Source:-1)
      at java.awt.Container.dispatchEventImpl(Unknown Source:-1)
      at java.awt.Component.dispatchEvent(Unknown Source:-1)
      at java.awt.KeyboardFocusManager.redispatchEvent(Unknown Source:-1)
      at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(Unknown Source:-1)
      at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(Unknown Source:-1)
      at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(Unknown Source:-1)
      at java.awt.DefaultKeyboardFocusManager.dispatchEvent(Unknown Source:-1)
      at java.awt.Component.dispatchEventImpl(Unknown Source:-1)
      at java.awt.Container.dispatchEventImpl(Unknown Source:-1)
      at java.awt.Window.dispatchEventImpl(Unknown Source:-1)
      at java.awt.Component.dispatchEvent(Unknown Source:-1)
      at java.awt.EventQueue.dispatchEvent(Unknown Source:-1)
      at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source:-1)
      at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source:-1)
      at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source:-1)
      at java.awt.EventDispatchThread.pumpEvents(Unknown Source:-1)
      at java.awt.EventDispatchThread.pumpEvents(Unknown Source:-1)
      at java.awt.EventDispatchThread.run(Unknown Source:-1)

Here is the TransactionalSystemImpl.executeImpl method:

private synchronized Object executeImpl(Transaction xact, boolean commit) {
    final Object result;

    try {
        if (commit) { // this is line 134
            clock.latch();
            synchronized(pendingEntries) {
                if (xactLatchCount > 0) {
                    pendingEntries.add(xact);
                } else {
                    xactLog.write(new TransactionEntry(xact, clock.time()));
                }
            }
        }

        final TransactionExecutor executor = transactionExecutorFactory.create(
                xact.getClass().getSimpleName()
        );

        if (executor == null) {
            throw new IllegalStateException("Failed to create transaction executor for transaction: " + xact.getClass().getName());
        }

        result = executor.execute(xact);

    } finally {
        if (commit) clock.unlatch();
    }

    return result;
}

Does anyone know what's going on here or how to fix it?

Answer 1

Looking for an answer drawing from credible and/or official sources.

Event Dispatch Thread and EventQueue

Swing event handling code runs on a special thread known as the Event Dispatch Thread(EDT). Most code that invokes Swing methods also runs on this thread. This is necessary because most Swing object methods are not thread safe. All GUI related task, any update should be made to GUI while painting process must happen on the EDT, which involves wrapping the request in an event and processing it onto the EventQueue. Then the event are dispatched from the same queue in the one by one in order they en-queued, FIRST IN FIRST OUT. That is, if Event A is enqueued to the EventQueue before Event B then event B will not be dispatched before event A.

SwingUtilities class has two useful function to help with GUI rendering task:

• invokeLater(Runnable): Causes doRun.run() to be executed asynchronously on the AWT event dispatching thread(EDT). This will happen after all pending AWT events have been processed, as is described above.
• invokeAndWait(Runnable): It has the same function as the invokeLater but it differs from invokeLater for the fact that:
  1. invokeAndWait waits for the task given by it to the EDT, to finish before returning.
  2. it blocks(awaits) the current(i.e., it's invoking thread from continuing it's execution by sending to WAIT state by means of synchronizing a lock.
  3. It will release the lock once the event request posted by this function gets dispatched in the EDT and the invoker thread of this function can continue.

The source code has the evidence:

  public static void invokeAndWait(Runnable runnable)
    throws InterruptedException, InvocationTargetException {

     if (EventQueue.isDispatchThread()) 
          throw new Error("Cannot call invokeAndWait from the event dispatcher thread");

     class AWTInvocationLock {}
     Object lock = new AWTInvocationLock();

     InvocationEvent event = new InvocationEvent(Toolkit.getDefaultToolkit(), 
                                                 runnable, lock,
                                                 true);

     synchronized (lock) {  //<<---- locking
           Toolkit.getEventQueue().postEvent(event);
           while (!event.isDispatched()) { //<---- checking if the event is dispatched
                  lock.wait(); //<---- if not tell the current invoking thread to wait 
              }
            }

      Throwable eventThrowable = event.getThrowable();
      if (eventThrowable != null) {
          throw new InvocationTargetException(eventThrowable);
      }
  }

This explains the issue:

My-Thread has just called java.awt.EventQueue.invokeAndWait(). So AWT-EventQueue-0 blocks My-Thread (I believe).

To explain a scenario of deadlock which you are likely having, lets look into an example:

class ExampleClass 
{

    public synchronized void renderInEDT(final Thread t)
    {
            try {
                SwingUtilities.invokeAndWait(new Runnable() {

                    @Override
                    public void run() {
                        System.out.println("Executinf invokeWait's Runnable ");
                        System.out.println("invokeWait invoking Thread's state: "+t.getState());

                        doOtherJob();
                    }
                });
            } catch (InterruptedException ex) {
                Logger.getLogger(SwingUtilitiesTest.class.getName()).log(Level.SEVERE, null, ex);
            } catch (InvocationTargetException ex) {
                Logger.getLogger(SwingUtilitiesTest.class.getName()).log(Level.SEVERE, null, ex);
            }

    }

    public synchronized void renderInEDT2(final Thread t)
    {
                SwingUtilities.invokeLater(new Runnable() {

                    @Override
                    public void run() {
                        System.out.println("Executing invokeLater's Runnable ");
                        System.out.println("invokeLater's invoking Thread's state: "+t.getState());
                        doOtherJob();
                    }
                });
        try {
            Thread.sleep(3000);
        } catch (InterruptedException ex) {
            Logger.getLogger(ExampleClass.class.getName()).log(Level.SEVERE, null, ex);
        }
    }


    public synchronized void doOtherJob()
    {
        System.out.println("Executing a job inside EDT");
    }
}

As you can see, i have declared three synchronized function:

1. renderInEDT(final Thread t): to execute a Runnable task in the EDT bySwingUtilities.invokeAndWait
2. renderInEDT2(final Thread t): to execute a Runnable task in the EDT by SwingUtilities.invokeLater
3. doOtherJob() : This function is invoked by each of the above two function's Runnable'S run() method.

The reference of the invoking thread is passed to check the state for each of the invocation of SwingUtilities function. Now, if we invoke renderInEDT() on an instance exmpleClass of ExampleClass: Here Thread t is declared in the class context:

    t =  new Thread("TestThread"){
                 @Override
                 public void run() {
                    exmpleClass.renderInEDT(t);
                 }

    };
    t.start();

The output will be:

Executing invokeWait's Runnable 
invokeWait invoking Thread's state: WAITING

The doOtherJob() method never gets executed in the EDT posted by SwingUtilities.invokeAndWait because a deadlock situation appears. As the renderInEDT() is synchronized and being executed inside a thread namely t, the EDT will need to wait to execute doOtherJob() until first invoking thread is done executing the renderInEDT(final Thread t) method, as is described in the official tutorial source of synchronized method:

it is not possible for two invocations of synchronized methods on the same object to interleave. When one thread is executing a synchronized method for an object, all other threads that invoke synchronized methods for the same object block (suspend execution) until the first thread is done with the object.

Hence, EDT is waiting for the t thread to finish(and suspend) execution, but thread t is actually blocked and sent to waiting state by the SwingUtilities.invokeAndWait method as described above, hence it is not being able to finish it's execution: Bth of the EDT and the t thread is waiting for each other to be done with their execution.

Let us see the above example with case: if we post event task using SwingUtilities.invokeLater as it will be evident if we execute the renderInEDT2() function on the exampleClass instance from a thread:

  t =  new Thread("TestThread"){
                 @Override
                 public void run() {
                    exmpleClass.renderInEDT2(t);
                 }   
    };
  t.start();

This time you will see that, the function invocation continues normally producing following output:

Executing invokeLater's Runnable 
invokeLater's invoking Thread's state: TIMED_WAITING
Executing a job inside EDT

This time doOtherJob() gets executed by the EDT as soon as the first calling thread of renderInEDT2() is done executing it: to emphasize i have put the thread to sleep (3s) to inspect the execution timing and hence it shows the state TIMED_WAITING.

This is what explains your second issue: As the exception is telling and also mentioned by you in one of your comment:

enderOnEDT is synchronized on something way up in the call stack, the com.acme.persistence.TransactionalSystemImpl.executeImpl method which is synchronized. And renderOnEDT is waiting to enter that same method. So, that is the source of the deadlock it looks like. Now I have to figure out how to fix it.

However,

• SwingUtilities.invokeAndWait(Runnable) especially used while we want to block or awaits a thread and ask user confirmation if we should continue using JOptionPane/JDialogue/JFileChooser etc.
• Otherwise, to post a GUI rendering task in the EventQueue, use SwingUtilities.invokeLater(Runnable).
• Although, i have await the EDT using Thread.sleep(time) for demonstration purposes, Please don't do anything such, which might block the EDT for a mentionable amount of time even if it is few, otherwise your Swing will get frozen and will force you to kill it.
• We should not perform any kind of computational work or read/write operation or any other thing that is not related to GUI rendering task inside EDT aka, inside the Runnable of the invokeAndWait and invokeLater function.

At this point you yourself should be able to figure out and solve your problem because you are not providing enough details to us of your code. I think while posting your GUI rendering task to Event Queue using a synchronized function like enderOnEDT as you have said in your comment, then I don't see any reason for calling another synchronized function from it's Runnable. Rather put your rendering function in this Runnable directly. That is my sole purpose of explaining the Event Queue and EDT mechanism.

Reference:

1. The Event Dispatch Thread
2. Class EventQueue
3. Initial Threads
4. SwingUtilities.invokeAndWait(Runnable doRun) documentation

Answer 2

It seems to be well known among Swing developers of my acquaintance that invokeAndWait is problematic, but maybe this isn't as well known as I had thought. I seem to recall having seen stern warnings in the documentation about difficulties in using invokeAndWait properly, yet I'm having a hard time finding anything. I cannot find anything in current, official documentation. The only thing I've been able to find is this line from an old version of the Swing Tutorial from 2005: (web archive)

If you use invokeAndWait, make sure that the thread that calls invokeAndWait does not hold any locks that other threads might need while the call is occurring.

Unfortunately this line seems to have disappeared from the current Swing tutorial. Even this is rather an understatement; I'd have preferred that it say something like, "If you use invokeAndWait, the thread that calls invokeAndWait must not hold any locks that other threads might need while the call is occurring." In general it's difficult to know what locks other threads might need during any given time, the safest policy is probably to ensure that the thread calling invokeAndWait doesn't hold any locks at all.

(This is pretty difficult to do, and it's why I said above that invokeAndWait is problematic. I also know that the designers of JavaFX -- essentially a Swing replacement -- defined in the javafx.application.Platform class a method called runLater which is functionally equivalent to invokeLater. But they deliberately omitted an equivalent method to invokeAndWait because it's very difficult to use properly.)

The reason is fairly straightforward to derive from first principles. Consider a system similar to the one described by the OP, having two threads: MyThread and the Event Dispatch Thread (EDT). MyThread takes a lock on object L and then calls invokeAndWait. This posts event E1 and waits for it to be processed by the EDT. Suppose that E1's handler needs to lock L. When the EDT processes event E1, it attempts to take the lock on L. This lock is held already by MyThread, which won't relinquish it until the EDT processes E1, but that processing is blocked by MyThread. Thus we have deadlock.

Here's a variation on this scenario. Suppose we ensure that processing E1 doesn't require locking L. Will this be safe? No. The problem can still occur if, just before MyThread calls invokeAndWait, an event E0 is posted to the event queue, and E0's handler requires locking on L. As before, MyThread holds the lock on L, so processing of E0 is blocked. E1 is behind E0 in the event queue so processing of E1 is blocked too. Since MyThread is waiting for E1 to be processed, and it's blocked by E0, which in turn is blocked waiting for MyThread to relinquish the lock on L, we have deadlock again.

This sounds fairly similar to what's going on in the OP's application. According to the OP's comments on this answer,

Yes, renderOnEDT is synchronized on something way up in the call stack, the com.acme.persistence.TransactionalSystemImpl.executeImpl method which is synchronized. And renderOnEDT is waiting to enter that same method. So, that is the source of the deadlock it looks like. Now I have to figure out how to fix it.

We don't have a complete picture, but this is probably enough to go on. renderOnEDT is being called from MyThread, which is holding a lock on something while it's blocked in invokeAndWait. It's waiting for an event to be processed by the EDT, but we can see the EDT is blocked on something held by MyThread. We can't quite tell exactly which object this is, but it kind of doesn't matter -- the EDT is clearly blocked on a lock held by MyThread, and MyThread is clearly waiting for the EDT to process an event: thus, deadlock.

Note also that we can be fairly sure the EDT isn't currently processing the event posted by invokeAndWait (analogous to E1 in my scenario above). If it were, the deadlock would occur every time. It seems to occur only sometimes, and according to a comment from the OP on this answer, when the user is typing quickly. So I'd bet that the event currently being processed by the EDT is a keystroke that happened to be posted to the event queue after MyThread took its lock, but before MyThread called invokeAndWait to post E1 to the event queue, thus it's analogous to E0 in my scenario above.

So far, this is probably mostly a recap of the problem, pieced together from other answers and from the OP's comments on those answers. Before we proceed to talking about a solution, here are some assumptions I'm making about the OP's application:

• It's multi-threaded, so various objects must be synchronized to work properly. This includes calls from Swing event handlers, which presumably update some model based on user interaction, and this model is also processed by worker threads such as MyThread. Therefore, they must lock such objects properly. Removing synchronization will definitely avoid deadlocks, but other bugs will creep in as the data structures are corrupted by unsynchronized concurrent access.

• The application isn't necessarily performing long-running operations on the EDT. This is a typical problem with GUI apps but it doesn't seem to be happening here. I'm assuming that the application works fine in most cases, where an event processed on the EDT grabs a lock, updates something, then releases the lock. The problem occurs when it can't get the lock because the lock's holder is deadlocked on the EDT.

• Changing invokeAndWait to invokeLater isn't an option. The OP has said that doing so causes other problems. This isn't surprising, as that change causes execution to occur in a different order, so it will give different results. I'll assume they would be unacceptable.

If we can't remove locks, and we can't change to invokeLater, we're left with calling invokeAndWait safely. And "safely" means relinquishing locks before calling it. This might be arbitrarily hard to do given the organization of the OP's application, but I think it's the only way to proceed.

Let's look at what MyThread is doing. This is much simplified, as there are probably a bunch of intervening method calls on the stack, but fundamentally it's something like this:

synchronized (someObject) {
    // code block 1
    SwingUtilities.invokeAndWait(handler);
    // code block 2
}

The problem occurs when some event sneaks in the queue in front of handler, and that event's processing requires locking someObject. How can we avoid this problem? You can't relinquish one of Java's built-in monitor locks within a synchronized block, so you have to close the block, make your call, and open it again:

synchronized (someObject) {
    // code block 1
}

SwingUtilities.invokeAndWait(handler);

synchronized (someObject) {
    // code block 2
}

This could be arbitrarily difficult if the lock on someObject is taken fairly far up the call stack from the call to invokeAndWait, but I think doing this refactoring is unavoidable.

There are other pitfalls, too. If code block 2 depends on some state loaded by code block 1, that state might be out of date by the time code block 2 takes the lock again. This implies that code block 2 must reload any state from the synchronized object. It mustn't make any assumptions based on results from code block 1, since those results might be out of date.

Here's another issue. Suppose the handler being run by invokeAndWait requires some state loaded from the shared object, for example,

synchronized (someObject) {
    // code block 1
    SwingUtilities.invokeAndWait(handler(state1, state2));
    // code block 2
}

You couldn't just migrate the invokeAndWait call out of the synchronized block, since that would require unsynchronized access getting state1 and state2. What you have to do instead is to load this state into local variables while within the lock, then make the call using those locals after releasing the lock. Something like:

int localState1;
String localState2;
synchronized (someObject) {
    // code block 1
    localState1 = state1;
    localState2 = state2;
}

SwingUtilities.invokeAndWait(handler(localState1, localState2));

synchronized (someObject) {
    // code block 2
}

The technique of making calls after having released locks is called the open call technique. See Doug Lea, Concurrent Programming in Java (2nd edition), sec 2.4.1.3. There is also a good discussion of this technique in Goetz et. al., Java Concurrency In Practice, sec 10.1.4. In fact all of section 10.1 covers deadlock fairly thoroughly; I recommend it highly.

In summary, I believe that using techniques I describe above, or in the books cited, will solve this deadlock problem correctly and safely. However, I am sure that it will require a lot of careful analysis and difficult restructuring as well. I don't see an alternative, though.

(Finally, I should say that while I am an employee of Oracle, this is not in any way an official statement of Oracle.)

---

UPDATE

I thought of a couple more potential refactorings that might help solve the problem. Let's reconsider the original schema of the code:

synchronized (someObject) {
    // code block 1
    SwingUtilities.invokeAndWait(handler);
    // code block 2
}

This executes code block 1, handler, and code block 2 in order. If we were to change the invokeAndWait call to invokeLater, the handler would be executed after code block 2. One can easily see that would be a problem for the application. Instead, how about we move code block 2 into the invokeAndWait so that it executes in the right order, but still on the event thread?

synchronized (someObject) {
    // code block 1
}

SwingUtilities.invokeAndWait(Runnable {
    synchronized (someObject) {
        handler();
        // code block 2
    }
});

Here's another approach. I don't know exactly what the handler passed to invokeAndWait is intended to do. But one reason it might need to be invokeAndWait is that it reads some information out of the GUI and then uses this to update the shared state. This has to be on the EDT, since it interacts with GUI objects, and invokeLater can't be used since it would occur in the wrong order. This suggests calling invokeAndWait before doing other processing in order to read information out of the GUI into a temporary area, then use this temporary area to perform continued processing:

TempState tempState;
SwingUtilities.invokeAndWait(Runnable() {
    synchronized (someObject) {
        handler();
        tempState.update();
    }
);

synchronized (someObject) {
    // code block 1
    // instead of invokeAndWait, use tempState from above
    // code block 2
}

Answer 3

It's hard to tell without seeing the code, but from the stack trace, it looks like you're firing some sort of transactional code from the event dispatch thread. Does that code then kick off an instance of My-Thread? The EDT could be blocked waiting for My-Thread from within the transactional code, but My-Thread can't finish because it needs the EDT.

If this is the case, you can use SwingUtilities.invokeLater for your rendering so the EDT finishes the transactional code and then it will render the updates. Or, you can not perform the transactional code from the EDT. For actual work that's not related to rendering, you should use a SwingWorker to avoid doing any heavy processing on the EDT.

Answer 4

I suspect the line 134 you quoted is not the real line 134 (can be caused by stale code, or some other issues). It seems that 134 is waiting for a monitor, which most probably means synchronized(pendingEntries), (or the clock.latch() which I think it is some kind of countdown latch?)

From the stack trace, the AWT event dispatching thread is waiting for a monitor, which is held by MyThread.

Please check the code base on the stack trace of MyThread. I believe somewhere it sync on pendingEntries, then it used invokeAndWait to ask the event dispatching thread to do something, and in turn the event dispatching thread is waiting for pendingEntries, which caused the deadlock.

---

A suggestion that is a bit off topics: Your event dispatching thread seems doing a lot more than it should. I don't think doing those transaction handling etc in the event dispatching thread is a good choice. Such action can be slow (and in this case, even blocks the event dispatching thread), which will cause UI to be unresponsive.

Splitting such action to a separate thread/executor seems a better choice for me.

Answer 5

Some thread (I assume My-Thread@101c) is synchronized on your TransactionalSystemImpl instance. The UI thread is trying to enter executeImpl but is blocked on the synchronized monitor and cannot. Where else is the TransactionalSystemImpl instance being used (with synchronized entry)? Probably between

  at com.acme.ui.ViewBuilder.renderOnEDT(ViewBuilder.java:157)
    .
    .
    .
  at com.acme.util.Job.run(Job.java:425)

Answer 6

If there are no other deadlocks running around, you can transform the call to EventQueue.invokeLater(Runnable) into a blocking version that waits until your Runnable is completed:

if (EventQueue.isDispatchThread()) r.run();
else {
    final Lock lock = new ReentrantLock();
    final AtomicBoolean locked = new AtomicBoolean(true);
    final Condition condition = lock.newCondition();

    EventQueue.invokeLater(() -> {
        r.run();
        try {
            lock.lock();
            locked.set(false);
            condition.signalAll();
        } finally {
            lock.unlock();
        }
    });

    try {
        lock.lock();
        while (locked.get())
            condition.await();
    } finally {
        lock.unlock();
    }
}