ArrayList throwing `ConcurrentModificationException` when trying to run `.size()` method

Question

Update

As was pointed out by Jiri Tousek, the error that was being thrown in my code has misled many amateur (and experienced) Java developers. Contrary to what the name seems to imply, ConcurrentModificationException does not have anything to do with multi-threading. Consider the following code:

import java.util.List;
import java.util.ArrayList;

class Main {
  public static void main(String[] args) {
    List<String> originalArray = new ArrayList<>();
    originalArray.add("foo");
    originalArray.add("bar");
    List<String> arraySlice = originalArray.subList(0, 1);
    originalArray.remove(0);
    System.out.println(Integer.toString(arraySlice.size()));
  }
}

This will throw a ConcurrentModificationException despite there being no threading involved.

The misleading exception name led me to think my problem was the result of how I was handling multi-threading. I've updated the title of my post with the actual issue.

Original (title: How to inform Java that you are finished modifying an ArrayList in a thread?)

I have code that looks roughly like the following:

class MessageQueue {
    private List<String> messages = new ArrayList<>();
    private List<String> messagesInFlight = new ArrayList<>();

    public void add(String message) {
        messages.add(message);
    }

    public void send() {
        if (messagesInFlight.size() > 0) {
            // Wait for previous request to finish
            return;
        }

        messagesInFlight = messages.subList(0, Math.min(messages.size, 10));
        for( int i = 0; i < messagesInFlight.size(); i++ )
        {
            messages.remove(0);
        }

        sendViaHTTP(messagesInFlight, new Callback() {
            @Override
            public void run() {
                messagesInFlight.clear();
            }
        });
    }
}

This is utilized in my code for analytics purposes. Every 10 seconds I call messageQueue.send() from a timer, and whenever an event of interest occurs I call messageQueue.add(). This class works *for the most part* -- I can add messages and they get sent via HTTP, and when the HTTP request completes the callback is run

The issue lies on the second tick of the timer. When I hit the line if (messagesInFlight.size() > 0) {, I get the following error:

java.util.ConcurrentModificationException
        at java.util.ArrayList$SubList.size(ArrayList.java:1057)

It seems like I can't read the .size() of the array in one thread (the second timer's callback) because it thinks the array is still being modified by the other thread (the first timer's callback). However I would expect the first timer's thread was destroyed and cleaned up after my call to sendViaHTTP, since there was no additional code for it to execute. Furthermore, the HTTP request is completing within 500 milliseconds, so a full 9.5 seconds passes without anything touching the empty messagesInFlight array

Is there a way to say say "hey, I'm done modifying this array, people can safely read it now"? Or perhaps a better way to organize my code?

Answer 1

The most glaring issue you have there is that you're using ArrayList.subList() while you don't seem to understand what it really does:

Returns a view of the portion of this list ... The returned list is backed by this list.

What you're storing in messagesInFlight is a view, not a copy. When you delete the first messages from messages, you're in fact deleting the very messages you had in your messagesInFlight right after subList() call. So after the for loop, there will be completely different messages, and the first n messages will be completely lost.

---

As to why you are getting the error you see - subList() specifically allows non-structural changes to both sub-list and the original list (non-structural means replacing the elements, not adding or removing them), and the example in the documentation also showcases how the original list may be modified by modifying the sub-list. However, modifying the original list and then accessing it through the sub-list is not allowed and may result in ConcurrentModifcationException, similarly to what happens when you change a list you're iterating through with an iterator.

Answer 2

If you need to be sure method send will not be called if previous call was not finished you can drop messages in fly collection and just use flag variable:

private AtomicBoolean inProgress = new AtomicBoolean(false);

public void send() {

    if(inProgress.getAndSet(true)) {
        return;
    }

    // Your logic here ...  

    sendViaHTTP(messagesInFlight, new Callback() {
        @Override
        public void run() {

            inProgress.set(false);
        }
    });

}