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)
            ...
    }
}

Answer 1

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.