Is ArrayList.add(int index, E element) thread unsafe?

Question

My Previous question brought me to this question.

Is add function of ArrayList thread safe?

I made a sample application with following classes

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


public class ThreadTest
{
    public static List<DummyObject> list = null;
    public static boolean isLoaded = false;
   public static void main(String [] args)
   {
      MyThread t1 = new MyThread(1);
      MyThread t2 = new MyThread(2);

      t1.start();
      t2.start();
   }

   public static void loadObject(){
       if(isLoaded){
           return;
       }
       isLoaded = false;
       try{
       list = new ArrayList<DummyObject>();
       for(int i=0;i<10;i++){
           list.add(i,new DummyObject());
       }}
       catch(Exception e){
           e.printStackTrace();
       }
       isLoaded = true;
   }
}

These are my threads

public class MyThread extends Thread
{
   int threadNumber ;
   public MyThread(int threadNumber)
   {
      this.threadNumber = threadNumber;
   }

   @Override
   public void run()
   {
       try {
        sleep(10-threadNumber);
    } catch (InterruptedException e1) {
        // TODO Auto-generated catch block
        e1.printStackTrace();
    }
     System.out.println("Running Thread: " + threadNumber);
     ThreadTest.loadObject();
     if(ThreadTest.isLoaded){
         System.out.println(ThreadTest.list);
         for(int i=0;i<ThreadTest.list.size();i++){
             if(ThreadTest.list.get(i)==null){
                 throw new NullPointerException();
             } 
         }
     }else {
         try {
                sleep(10);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
         }
   }
}

This is my dummy class

public class DummyObject {

}

Even though I wasn't able to replicate the Null Pointer Exception that I got on my previous question , I sometimes get this error

Exception in thread "Thread-1" java.lang.IndexOutOfBoundsException: Index: 1, Size: 10
    at java.util.ArrayList.add(ArrayList.java:367)
    at ThreadTest.loadObject(ThreadTest.java:25)
    at MyThread.run(MyThread.java:20)

Form ArrayList Code this is the line thats throwing an error:

if (index > size || index < 0)
        throw new IndexOutOfBoundsException(
        "Index: "+index+", Size: "+size);

But as we can see from Exception index is 1 and size is 10, so there is no way that if condition is satisfied. So is my assumption correct that add function of arrayList is thread unsafe or is something else going on here?

ArrayList as a whole is not thread safe, so no, that method is probably not thread safe either. — Joachim Sauer
– Joachim Sauer, Commented Nov 16, 2012 at 9:16
You can't expect an improperly synchronized code to work when run concurrently. Arraylist is not thread safe and you don't synchronize acceses to your boolean flag. Even if Arraylist were thread safe you don't publish it properly and your threads might see it null. — assylias
– assylias, Commented Nov 16, 2012 at 9:22

didierc · Accepted Answer · 2012-11-16 09:27:55Z

4

From the documentation:

(This class is roughly equivalent to Vector, except that it is unsynchronized.)

You need to either implement the synchronization yourself, or better yet, use a synchronized container like Vector.

In the case of your code, you have 2 threads running the same piece of code (loadObject) in which several static values are accessed/modified. You need to make sure that each access is done in a synchronized manner. You have 2 threads, thus you allocate twice the ThreadTest.list field, so one of the allocation is useless, but more importantly, there might be some values inserted in that list before it is lost, so these values become lost as well.

You should make sure that the list is not allocated before allocating it.

You could also have problem with the isLoaded field, leading to more than 10 elements in your list.

edited Nov 16, 2012 at 9:27

answered Nov 16, 2012 at 9:13

didierc

14.8k3 gold badges35 silver badges52 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Joachim Sauer Over a year ago

Also note: synchronizing every method of a class does not necessarily make your whole code thread safe. Example: using indexOf() followed by set(int,Object) to replace the found object can produce unexpected results if the list is modified between the two calls (even on a fully synchronized List).

Ankur Over a year ago

thanks for the answer, then this should be the reason for unexpected behaviour

Joachim Sauer Over a year ago

@Ankur: the problem in your previous question surely comes from some concurrent work that's not properly synchronized. Unsynchronized Lists are probably your smallest problems. For example, I'd say that construction and use of such (static-like) Lists should be separate steps and you seem to run both at once, which is a bad idea. Simply sprinkling synchronize or calls to Colelctions.synchronizedList() over your code will not fix the underlying problem (it might hide some of the symptoms, however).

Joachim Sauer Over a year ago

@Ankur: the location of that code doesn't matter. If it runs at the same time, then you'll have to look carefully how to synchronize it. The safest (and possibly easiest) way to solve this: ensure that no thread other than the producing one can get access to the List until it is fully constructed.

Ankur Over a year ago

@JoachimSauer thanks will see to it that only one thread is adding object to the arrayList

|

EdC · Accepted Answer · 2012-11-16 09:25:09Z

Short answer, no, it's not thread safe. From the JavaDoc

Note that this implementation is not synchronized. If multiple threads access an ArrayList instance concurrently, and at least one of the threads modifies the list structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more elements, or explicitly resizes the backing array; merely setting the value of an element is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the list. If no such object exists, the list should be "wrapped" using the Collections.synchronizedList method. This is best done at creation time, to prevent accidental unsynchronized access to the list:
List list = Collections.synchronizedList(new ArrayList(...));

In general, none of the modern collections in Java are thread safe. If you simply want to make them not blow up you can use Collections.synchronizedList as suggested in the JavaDoc. However it's worth noting that this just means that only one thread can access the collection at at time. This does make it safe but can cause issues with threads being blocked.

If you're trying to get high concurrency then you really want to look at the java.util.concurrent package . This gives you nice classes like the ArrayBlockingQueue which makes this kind of thread hand off quite easy. Better still, have a look at the Executor implementations that can handle a lot of this complexity for you.

Bhavik Ambani · Accepted Answer · 2012-11-16 09:23:22Z

I have placed the actual code of the add method of ArrayList class.

/**
     * Inserts the specified element at the specified position in this
     * list. Shifts the element currently at that position (if any) and
     * any subsequent elements to the right (adds one to their indices).
     *
     * @param index index at which the specified element is to be inserted
     * @param element element to be inserted
     * @throws IndexOutOfBoundsException {@inheritDoc}
     */
    public void add(int index, E element) {
        rangeCheckForAdd(index);

        ensureCapacityInternal(size + 1);  // Increments modCount!!
        System.arraycopy(elementData, index, elementData, index + 1,
                         size - index);
        elementData[index] = element;
        size++;
    }

Here this method states that it is not thread safe, as the elementData is the

 private transient Object[] elementData;

,which is updated and in the multithreaded environments it can cause the vital problem.

Collectives™ on Stack Overflow

Is ArrayList.add(int index, E element) thread unsafe?

3 Answers 3

6 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related