Class managing queuing updates from another thread, Version 2

Question

This is an updated version of: Class to manage updates coming in from another thread

Managing updates from other threads into Swing is a hard problem. In MVC design, if you don't want to have the Presenter be responsible for Thread safety, you can end up with deadlock issues, and also too many little tasks getting started; not great.

I have written a class designed to manage these updates in the view so that the Presenter is not coupled with the View's threading model. It loads them all into a queue, and then, if a new update is added, it processes the entire queue in case too many updates have come too quickly.

Changes:

• I removed the race condition, I believe
• Removed the need for checking for InterruptedException, and therefore a LOG in this class
• Added a lock to ensure that all updates are properly processed
• Removed visibility of Runnable by making it an inner class
• changed name of setObject to postUpdate
• Removed unnecessary constructor
• Other minor cosmetic changes

Here's the result:

public abstract class SwingUpdater<E> {
    private final LinkedBlockingQueue<E> updates = new LinkedBlockingQueue<>();;
    private final AtomicBoolean updating = new AtomicBoolean(false);

    public final void postUpdate(E object) {
        updates.add(object);
        synchronized(updating) {
            if (!updating.getAndSet(true)) {
                EventQueue.invokeLater(new SwingUpdaterRunnable());
            }
        }
    }

    private class SwingUpdaterRunnable implements Runnable {
        @Override
        public void run() {
            Deque<E> pending = new LinkedList<>();

            do {
                updates.drainTo(pending);

                while (!pending.isEmpty()) {
                    doTask(pending.removeFirst());
                }
            } while (!atomicCheckCanStop());
        }

        private boolean atomicCheckCanStop() {
            synchronized(updating) {
                boolean isEmpty = updates.isEmpty();
                if(isEmpty) {
                    updating.set(false);
                }
                return isEmpty;
            }
        }
    }

    protected abstract void doTask(E update);
}

For convenience, here's the sample main again to demonstrate this class in action:

import java.awt.*;
import java.util.Random;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import javax.swing.*;

public class SwingUpdaterExample {
    private final JFrame testFrame;
    private final JPanel contentPane;
    private final JTextPane textPane;
    private int updateCount;

    private final SwingUpdater<String> updater = new SwingUpdater<String>() {
        @Override
        protected void doTask(String update) {
            String oldText = textPane.getText();
            textPane.setText(oldText + ((updateCount & 3) == 0 ? System.lineSeparator() : "\t") + update);
            updateCount++;
        }
    };

    public SwingUpdaterExample() {
        textPane = new JTextPane();

        contentPane = new JPanel(new BorderLayout());
        contentPane.add(textPane);

        testFrame = new JFrame("ExampleFrame");
        testFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        testFrame.setSize(500,800);
        testFrame.setContentPane(contentPane);
    }

    public void setVisible(boolean visible) {
        testFrame.setVisible(true);
    }

    public void appendText(String string) {
        updater.postUpdate(string);
    }

    public static class ExampleTask implements Runnable {
        private final String name;
        private final SwingUpdaterExample view;

        public ExampleTask(String name, SwingUpdaterExample view) {
            this.name = name;
            this.view = view;
        }

        @Override
        public void run() {
            Random r = new Random();
            for(int counter = 0; counter < 15; counter++) {
                if(r.nextDouble() < 0.3) {
                    try {
                        Thread.sleep(2000 + r.nextInt(4000));
                    } catch (InterruptedException e) {
                        break;
                    }
                }
                view.appendText(name + " - " + counter);    
            }
        }
    }

    public static void main(String[] args) throws Exception {
        final SwingUpdaterExample frame = new SwingUpdaterExample();
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                frame.setVisible(true);
            }

        });

        ExecutorService service = Executors.newCachedThreadPool();
        for(int i = 0; i < 4; i++) {
            ExampleTask task = new ExampleTask("Task" + i, frame);
            service.submit(task);
        }

        service.shutdown();
    }
}

Answer 1

Synchronization and atomic boolean is overkill. Especially since you only access the boolean inside the synchronized blocks. Choose one or the other.

Draining to a LinkedList and then removeFirst each element is less efficient than reserving enough space in a ArrayList, draining to it and iterating over it.

do {
    ArrayList<E> pending = new ArrayList<E>(updates.size()+3);
    updates.drainTo(pending);

    for(E task : pending){
        doTask(task);
    }

} while (!atomicCheckCanStop());

---

Having said all that the Java devs have solved this problem before and created sun.swing.AccumulativeRunnable which is used by SwingWorker for it's publish->process mechanism and grouping the setProgress() calls into 1 event. There they don't bother with checking the queue again after draining it.