Java - Thread safety of ArrayList constructors

I am looking at this piece of code. This constructor delegates to the native method "System.arraycopy"

Is it Thread safe? And by that I mean can it ever throw a ConcurrentModificationException?

public Collection<Object> getConnections(Collection<Object> someCollection) {
    return new ArrayList<Object>(someCollection);
}

Does it make any difference if the collection being copied is ThreadSafe eg a CopyOnWriteArrayList?

public Collection<Object> getConnections(CopyOnWriteArrayList<Object> someCollection) {
    return new ArrayList<Object>(someCollection);
}

Edit: I am aware that Thre开发者_StackOverflow中文版adSafe != ConcurrentModificationException. I am trying to take a snapshot of data at a point in time. Therefore if another Thread writes to someCollection midway thru the copy I dont care if the result has the new object or not. I just dont want it to throw a ConcurrentModificationException or worse

This constructor delegates to the native method "System.arraycopy"

Actually, it calls toArray() on someCollection. That will eventually call System.arraycopy if someCollection is an ArrayList. For other collection types the array will be created in other ways.

Is it Thread safe?

No.

And by that I mean can it ever throw a ConcurrentModificationException?

If it is an ArrayList it won't throw ConcurrentModificationException ... but that does not make it thread-safe!!

I am trying to take a snapshot of data at a point in time.

You won't (always) get a consistent snapshot.

For example, if a different thread calls set(obj, pos) on someCollection while your thread is calling this constructor, then the contents of your newly created ArrayList are unpredictable.

In the Java 11 version, the ArrayList(Collection) constructor calls toArray() on the argument collection. The resulting ArrayList will be a consistent snapshot of the collection if and only if the toArray call is guaranteed to give a consistent snapshot. This is true for some collection classes (for example CopyOnWriteList) but not in general.

Your question is whether you can safely get a snapshot of a collection that might be undergoing concurrent modification by another thread using new ArrayList<Foo>(thatCollection). The answer is: as long as thatCollection itself is thread-safe, yes. So if it's a CopyOnWriteArrayList, synchronizedList or Vector, If it's not thread-safe, for example if it's another ArrayList, you're not fine. (What will happen could be worse than a ConcurrentModificationException.)

The reason is that the ArrayList constructor makes only a single atomic call to the other collection -- to its toArray method. So it essentially enjoys whatever thread-safety guarantees that method itself has. It was not always implemented like this, but is now for just this reason. We do the same thing in Guava with ImmutableList.copyOf.

Thread safe and ConcurrentModificationException are different concepts. A thread safe object is one where multiple threads can call its methods at the same time, and the data held within the object is guaranteed not to become corrupted (example: http://thejavacodemonkey.blogspot.com/2007/08/making-your-java-class-thread-safe.html). A ConcurrentModificationException occurs when, for example, you are in the middle of iterating through a collection, and the collection changes. The change might come from a different thread, or the same thread.

In your constructor, if another thread changes the someCollection while your constructor is copying it, it could result either in undefined behaviour (i.e. data corruption in your new collection, because the collections are not thread safe), or a ConcurrentModificationException (if the collection does detect the concurrent modification, but this is not guaranteed, because it is not thread safe... :-)

If your constructor is going to take a Collection<Object>, you would need to ensure that other threads do not modify the collection until after your constructor returns.

On the other hand, a CopyOnWriteArrayList is thread safe and guarantees not to throw ConcurrentModificationException, so you should be safe doing it this way, without writing extra synchronization code.

ConcurrentModificationException is not the only sign of something being not thread safe. If for example in method #1 someCollection is also an ArrayList, you will never have a ConcurrentModificationException (see the code). However, data integrity is at risk - if the source ArrayList is altered during its copying, only some of the changes may be reflect in the copy (and not necessarily the oldest changes!).

In other words, atomicity is not guaranteed (unless the source is specifically designed for it, such as with CopyOnWriteArrayList).

EDIT: actually, assumning that you don't synchronize your threads properly, copying an array with one thread while another thread updates references in it is problematic in the Java Memory Model, and theoretically can result in unpredictable behavior.