Do I need to synchronize access to a List that is only modified by one thread?

Question

Here I have a class that has two threads that have access to a List. One thread periodically replaces the list with an updated copy, and the other thread paints the list's contents onto the screen.

public class ThreadSafePainter {
    private List<String> dataList = new ArrayList<>();

    /*
     *  starts a thread to periodically update the dataList
     */
    public ThreadSafePainter() {
        Thread thread = new Thread(() -> {
            while (true) {
                // replace out-dated list with the updated data
                this.dataList = getUpdatedData();
                // wait a few seconds before updating again
                Thread.sleep(5000);
            }
        });
        thread.start();
    }

    /*
     *  called 10 times/second from a separate paint thread
     *  Q: Does access to dataList need to be synchronized?
     */
    public void onPaint(Graphics2D g) {
        Point p = new Point(20, 20);

        // iterate through the data and display it on-screen
        for (String data : dataList) {
            g.drawString(data, p.x, p.y);
            p.translate(0, 20);
        }
    }

    /*
     *  time consuming data retrieval
     */
    private List<String> getUpdatedData() {
        List<String> data = new ArrayList<>();
        // retrieve external data and populate list
        return data;
    }
}

My question is, do I need to synchronize access to the dataList? How should I go about doing that? Would this work:

public ThreadSafePainter() {
    ...
            synchronized (this) {
                this.dataList = getUpdatedData();
            }
    ...
}

public void onPaint(Graphics2D g) {
    ...
    synchronized (this) {
        for (String data : dataList)
            ...
    }
}

Since `getUpdatedData()` creates a new list every time, you only need a safe publication. In this case, declaring the field `dataList` as `volatile` would be sufficient. It’s important that this works if the list reference gets stored after it has been populated and is never modified again (as the next update creates a new list) and that the reader reads the reference once per processing (like `for(…: dataList)` does). If it needs to access the list multiple times during one `paint`, it has to store it in a local variable then. — Holger, Oct 29 '15 at 20:24
Whenever two or more threads share any **mutable** state, there **must** be some kind of mechanism in place to handle concurrency. Whether it's low-level synchronization, higher level concurrency classes, `Atomic*` classes or `volatile` fields depends on the actual situation but something must always be put in place. — biziclop, Oct 29 '15 at 20:28
@biziclop your point being, in regard to this specific situation? — njzk2, Oct 29 '15 at 20:37
@njzk2 My point is giving OP a quick and accurate way of answering any "Do I need synchronisation in situation X?" type question. You know, give the man a fish and all that. — biziclop, Oct 29 '15 at 20:39
@StackFlowed no, this is just a very simplified version of the actual implementation, just trying to gain a better understanding of concurrency — Will, Oct 29 '15 at 21:15
@Holger thanks for the insight, hadn't thought of using volatile that may be more suited for this scenario — Will, Oct 29 '15 at 21:20
@SleimanJneidi a separate thread in charge of calling onPaint at a fixed rate — Will, Oct 29 '15 at 21:21
I agree with @Holger's assessment. Moreover, and this may be outside the scope of your question, but you seem to gloss over your implementation of getUpdatedData(). You need to make sure this is written to be thread-safe as well, which might involve synchronization or hand-off with a volatile. — JimN, Oct 30 '15 at 03:32

score 1 · Answer 1 · answered Sep 21 '16 at 20:53

Any time you have more than one thread accessing the same mutable state (well, almost anytime, there are some exceptions, like when you know the state won't mutate within the other thread's lifetime), you need to take some type of action. In this case, you are mutating the field dataList and you expect another thread to react to this. So, you need to do "something". The most general solution is to use synchronized, and your outline of how to do this is just fine.

If you want to use squeeze maximum performance out of something (which is kind of ridiculous for a GUI problem), or you want to show off your great understanding of concurrency, you can consider more light-weight alternatives that apply to more limited circumstances. In this case, you have only one writer, and the writer is only writing a single reference. For such cases, volatile is sufficient. In this kind of code, I would personally just stick to synchronized because it is less likely to break when you change the code, like perhaps you add another writer thread or something.

score 0 · Answer 2 · answered Oct 30 '15 at 00:29

0

If you do not make the list synchronized then you should make the List volatile. This way the reading thread gets the latest copy of the List variable's value.

Here is a good explanation.

answered Oct 30 '15 at 00:29

Jose Martinez

11,452
7
53
68

score -1 · Answer 3 · answered Oct 29 '15 at 20:31

The official Java-Documentation points out, that an ArrayList is not synchronized. So you need to synchronize it.

However the documentation also says this applies only if multiple threads access the same list. So in your case it should not be necessary to synchronize it. But if you want to be 100% sure you can synchronize your List with this simple call:

List<data_type> list = Collections.synchronizedList(new ArrayList<data_type>());

...where "data_type" is the type values you want to store.

That documentation is misleading. Even if the list doesn't need to be synchronized, you do need to publish the reference to the list in a safe manner. — biziclop, Oct 29 '15 at 20:33

Do I need to synchronize access to a List that is only modified by one thread?

3 Answers3