CODEWITHSUNDEEP
kotlincollectionsconcurrencyconcurrentmodification
user2808624
user2808624

Reputation: 2530

How can joinToString on a cloned ArrayList throw ConcurrentModificationException

I have an ArrayList of Strings, which is constantly changed (adding and removing lines) by a non-UI thread. In the UI I want to show the current content of that ArrayList in a simple TextView.

When preparing the text for the TextView by joinToString("\n"), I was immediately receiving a ConcurrentModificationException, which was not very surprising.

So, to avoid that, I cloned the ArrayList, before joining it to a String:

val textLines = globalTextLineArray.clone() as ArrayList<String>
myTextView.text = textLines.joinToString("\n")

What I am not understanding is, that even now I am receiving from time to time a ConcurrentModificationException.

How can that happen?

The StackTrace is:

java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.next(ArrayList.java:866)
    at kotlin.collections.CollectionsKt___CollectionsKt.joinTo(_Collections.kt:3341)
    at kotlin.collections.CollectionsKt___CollectionsKt.joinToString(_Collections.kt:3361)
    at kotlin.collections.CollectionsKt___CollectionsKt.joinToString$default(_Collections.kt:3360)
    at MyActivity.updateTextInView(MyActivity.kt:79)
    at MyActivity.access$updateTextInView(MyActivity.kt:24)
    at MyActivity$textViewUpdater$1.run(MyActivity.kt:85)
    at android.os.Handler.handleCallback(Handler.java:789)
    at android.os.Handler.dispatchMessage(Handler.java:98)
    at android.os.Looper.loop(Looper.java:164)
    at android.app.ActivityThread.main(ActivityThread.java:6944)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)

There may be better ways to achieve what I am trying to do (e.g. using a thread-safe collection). But still I would like to understand, why the cloning attempt does not work.

Upvotes: 2

Views: 515

Answers (1)

Sweeper
Sweeper

Reputation: 273978

You are not locking anything, so clone on the UI thread is executed at the same time as remove/add on the other thread. The code could be run in any order. This is bad, because you could cloning an array list that is in the middle of removing an element - in an intermediate, invalid state. When you try to iterate on that incorrectly cloned object, the iterator is confused and thinks you are modifying it concurrently.

In short, ArrayList is not thread safe. It does not make any guarantees. You seem to understand that joinToString isn't thread safe. Well, clone isn't either, and neither is add or remove.

To illustrate how this can happen, let's assume you're using OpenJDK 11. The ArrayList.remove method, among other things, decrease the size field:

// line 670
final int newSize;
if ((newSize = size - 1) > i)
    System.arraycopy(es, i + 1, es, i, newSize - i);
es[size = newSize] = null; // updates size field

Now let's look at what clone does:

// line 373
public Object clone() {
    try {
        ArrayList<?> v = (ArrayList<?>) super.clone();
        v.elementData = Arrays.copyOf(elementData, size);
        v.modCount = 0;
        return v;
    } catch (CloneNotSupportedException e) {
        // this shouldn't happen, since we are Cloneable
        throw new InternalError(e);
    }
}

Let's try to contrive a situation that will cause an invalid array list to be cloned.

First, it calls super.clone. Imagine that this happens before size = newSize, so the old size before the removal gets cloned. Then, clone calls Arrays.copyOf to copy the internal array. Note that in this line, it accesses size again. Imagine that this time, the access is done after size = newSize, so we get the new smaller size. What we have now is an array list with size > elementData.length!

What happens when you iterate over this? The iterator Itr implemented like this:

// line 990
public boolean hasNext() {
    return cursor != size;
}

@SuppressWarnings("unchecked")
public E next() {
    checkForComodification();
    int i = cursor;
    if (i >= size)
        throw new NoSuchElementException();
    Object[] elementData = ArrayList.this.elementData;
    if (i >= elementData.length)
        throw new ConcurrentModificationException();
    cursor = i + 1;
    return (E) elementData[lastRet = i];
}

It will keep iterator until cursor == size,and note how in next, it checks for i >= elementData.length. Remember that our size is one more than elementData.length, so this condition will eventually be satisfied.

When you modify the array list, you should lock it, and when you clone it or join it to string, you should also lock it.

Upvotes: 2

Related Questions