Creating a "Produce and Consume" using Swingworkers

I am experimenting with the produce and consume concept while trying to incorporate it into my own project.

I basically have two Workers in a small example I made:

ProduceWorker: Populates a list with random numbers with a delay to emulate a real world adding in the doInBackground() method. Then in the process() that list is copied to another list which is used in the second worker and the first list is cleared.

public class ProduceWorker extends SwingWorker> {

        @Override
        protected Void doInBackground() throws InterruptedException {
            while (true) {
                for (int i = 0; i > chunks) {
            jReadyNumber.setText(Integer.toString(chunks.get(0).size()));

            synchronized (_addingList) {
                synchronized (_editingList) {
                    _editingList = new ArrayList<>(_addingList);
                    _addingList.clear();
                }
            }
        }
    }

ConsumeWorker: Grabs the second list (copy of the first)and just emulates doing something in it's elements with a random delay in the doInBackground() method. Then in the process() that list is cleared.

public class ConsumeWorker extends SwingWorker> {

        @Override
        protected Void doInBackground() throws InterruptedException {
            while (true) {
                for (int number : _editingList) {
                    Thread.sleep(_rand.nextInt(100));
                }
                publish(_editingList);
            }
        }

        @Override
        protected void process(List> chunks) {
            jDoneNumber.setText(Integer.toString(chunks.get(0).size()));
            synchronized (_editingList) {
                _editingList.clear();
            }
        }
    }

Now I have this example in a JFrame with some labels and a toggle button that when it gets pressed, both workers start executing.

Here is the whole MainGui.java:

```
package gui;

import java.util.ArrayList;
impor

Don't lock on non-final objects.

The reason for this is that something later may change the variable. I might say _addingList = new ArrayList<>(); and then the synchronized blocks fail.

//ProduceWorker
    @Override
    protected void process(List> chunks) {
        jReadyNumber.setText(Integer.toString(chunks.get(0).size()));

        synchronized (_addingList) {
            synchronized (_editingList) {
                _editingList = new ArrayList<>(_addingList);
                _addingList.clear();
            }
        }
    }

    //ConsumeWorker
    @Override
    protected void process(List> chunks) {
        jDoneNumber.setText(Integer.toString(chunks.get(0).size()));
        synchronized (_editingList) {
            _editingList.clear();
        }
    }

What could happen here is the following:

• Thread A enters ConsumeWorker.process and yields.

• Thread B enters ProduceWorker.process and obtains a lock on _addingList and _editingList. Then it runs _editingList = new ArrayList<>(_addingList); and it yields.

• Thread A obtains a lock on _editingList (It can, because the instance changed) and runs _editingList.clear(). It ends the function and yields.

• Thread B runs _addingList.clear(), ends the function and yields.

You've just cleared both your lists.

I'm not sure that was the intended goal, but it could happen. Maybe it can't happen right now due to certain circumstances. But you'll make a future change and all of a sudden it will happen and you will have bugs and these bugs will be near impossible to reproduce... you'll get a gradually building list of tickets of customers who report that sometimes, it doesn't work.

Make a special final Object _addingListLock that you create once and never mess with again. Then synchronize on that. Do the same for _editingList.

StackExchange Code Review Q#60486, answer score: 5