views:

539

answers:

1

I'm debugging some code that was written using a SwingWorker to perform a mix of numerical calculation and GUI update. The SwingWorker hangs with the following stack trace :

Full thread dump Java HotSpot(TM) Client VM (14.3-b01 mixed mode, sharing):

"SwingWorker-pool-3-thread-4" prio=6 tid=0x07fd7c00 nid=0x143c waiting on condition [0x0a33f000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
        at java.lang.Thread.sleep(Native Method)
        at sum.ee.ui.modelviewer.ModelViewer$ModelAnimator.doInBackground(ModelViewer.java:940)
        at sum.ee.ui.modelviewer.ModelViewer$ModelAnimator.doInBackground(ModelViewer.java:877)
        at javax.swing.SwingWorker$1.call(SwingWorker.java:274)
        at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
        at java.util.concurrent.FutureTask.run(FutureTask.java:138)
        at javax.swing.SwingWorker.run(SwingWorker.java:313)
        at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
        at java.lang.Thread.run(Thread.java:619)

"SwingWorker-pool-3-thread-3" prio=6 tid=0x07fd7000 nid=0x11a8 waiting for monitor entry [0x0a2af000]
   java.lang.Thread.State: BLOCKED (on object monitor)
        at java.awt.Component.resize(Component.java:2044)
        - waiting to lock <0x24b936a0> (a java.awt.Component$AWTTreeLock)
        at java.awt.Component.setSize(Component.java:2035)
        at java.awt.Component.resize(Component.java:2069)
        at java.awt.Component.setSize(Component.java:2060)
        at javax.swing.JViewport.setViewSize(JViewport.java:1038)
        at javax.swing.ViewportLayout.layoutContainer(ViewportLayout.java:183)
        at java.awt.Container.layout(Container.java:1421)
        at java.awt.Container.doLayout(Container.java:1410)
        at jsyntaxpane.components.LineNumbersRuler.updateSize(LineNumbersRuler.java:109)
        at jsyntaxpane.components.LineNumbersRuler.removeUpdate(LineNumbersRuler.java:203)
        at javax.swing.text.AbstractDocument.fireRemoveUpdate(AbstractDocument.java:243)
        at jsyntaxpane.SyntaxDocument.fireRemoveUpdate(SyntaxDocument.java:118)
        at javax.swing.text.AbstractDocument.handleRemove(AbstractDocument.java:608)
        at javax.swing.text.AbstractDocument.remove(AbstractDocument.java:576)
        at javax.swing.JEditorPane.setText(JEditorPane.java:1493)
        at sum.ee.ui.SourceCodePanel.clearSourcePane(SourceCodePanel.java:256)
        at sum.ee.ui.SourceCodePanel.access$100(SourceCodePanel.java:47)
        at sum.ee.ui.SourceCodePanel$1.stateChanged(SourceCodePanel.java:209)
        at sum.ee.ui.VisualizationAggregator.fireStateChanged(VisualizationAggregator.java:300)
        at sum.ee.ui.VisualizationAggregator.update(VisualizationAggregator.java:97)
        at sum.ee.ui.modelviewer.ModelViewer$ModelAnimator.doInBackground(ModelViewer.java:918)
        at sum.ee.ui.modelviewer.ModelViewer$ModelAnimator.doInBackground(ModelViewer.java:877)
        at javax.swing.SwingWorker$1.call(SwingWorker.java:274)
        at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
        at java.util.concurrent.FutureTask.run(FutureTask.java:138)
        at javax.swing.SwingWorker.run(SwingWorker.java:313)
        at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
        at java.util.concurrent.ThreadPoolExecutor

$Worker.run(ThreadPoolExecutor.java:908) at java.lang.Thread.run(Thread.java:619)

My understanding is that GUI work should not be done inside of doInBackground(), but rather in done(). I performed a naive experiment where I moved all of the code in doInBackground() into done() and it still didn't work. Is there any tips that folks can give me on what I can do to root cause this issue? The code looks like this :

protected Void doInBackground() {
    isAnimating = true;
    resetButtonBackgrounds();
    backgroundColor = new Color(175, 255, 175);  // Soft Green

    JToggleButton b = null;
    for (final int index : modelIndices) {
        if (index == modelIndices.get(modelIndices.size() - 1)) {
            backgroundColor = defaultBackgroundColor;
        }
        if (!keepTrace) {
            // Resetting the backgrounds is necessary to have
            // individual display of the changing elements due to
            // the fact that there can be multiple nodes per
            // source line.  The reset works in combination
            // with updating from ModelViewer.this (as opposed
            // to the 'this' of ModelAnimator instances) due
            // to not sending an event to itself.  Furthermore,
            // if the event was sent from ModelAnimator, the model
            // indices are recalculated, causing a jump when multiple
            // element source lines are encountered.
            resetButtonBackgrounds();
        }
        aggregator.modelIndex(index);
        aggregator.update(ModelViewer.this);

        b = getButtonByIndex(index);
        scrollRectToVisible(b.getBounds());
        ModelViewer.this.repaint();
        try {
            StaticTools.sleepAtLeast(sleepTimeMilliseconds);
        } catch (final InterruptedException ex) {
            // continue with thread
        }
    }

    isAnimating = false;

    if ( b != null) {

        Color orig = b.getBackground();
        Color blink = Color.PINK;
        Color current = orig;
        for (int i = 0; i < 100; i++) {

            try {
                Thread.sleep(100);
            } catch (InterruptedException ex) {
            }

            if (current == orig) {
                current = blink;
            } else {
                current = orig;
            }

            b.setBackground(current);
            ModelViewer.this.repaint();

        }
    }

    return null;
}

The other clue is that there are two SwingWorker threads that execute. Can they be running the same thread?

UPDATE: Here is the code that executes the SwingWorker :

public final void animate(final long delayBetweenUpdatesMilliseconds, final List modelIndices, final boolean keepTrace, final List propertyChangeListeners) {

ModelAnimator modelAnimator =
        new ModelAnimator(delayBetweenUpdatesMilliseconds, modelIndices,
        keepTrace);
for (final PropertyChangeListener listener : propertyChangeListeners) {
    modelAnimator.addPropertyChangeListener(listener);
}

modelAnimator.execute();

}

+1  A: 

This is a failure to observe the Swing EDT rules.

The purpose of SwingWorker is to do heavy nonGUI tasks when a UI event occurs that would otherwise block the UI, then update the UI at the end.

So you would implement your weight lifting inside doInBackground(); then once finished swing would call done() on the EDT and you can retrieve the results using get().

The problem here is you are doing GUI work in the new thread SwingWorker has created. This can lead to deadlocks and concurrency issues.

This includes the creation of said GUI objects, which should be in runnables even if your already on the EDT

Actions such as:

b = getButtonByIndex(index);

Should be encased in a Runnable with InvokeandWait. Stuff that actually modifies the GUI itself especially need to be in their own runnables, even if your already in the swing event dispatch thread responding to a button press or change, you run the risk of working on objects your already working on.

e.g. swing is working on and locked A to let you do work, letting you do work on B attempting to lock and work on A

Tom J Nowell