Thread-safe arraylist with get, put, add, remove functions

Question

I am preparing for multithreading interview questions and I don't have much experience with multithreading. I referred some online article to come up with a solution. Is this solution appropriate?

public class ThreadSafeArrayList<E> {
 private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
 private final Lock readLock = readWriteLock.readLock();
 private final Lock writeLock = readWriteLock.writeLock();
 private final List<E> list = new ArrayList<>();
 public void set(E o) {
 writeLock.lock();
 try {
 list.add(o);
 } finally {
 writeLock.unlock();
 }
 }
 public E get(int i) {
 readLock.lock();
 try {
 return list.get(i);
 } finally {
 readLock.unlock();
 }
 }
 public void add(E num) {
 List<E> sync = Collections.synchronizedList(list);
 sync.add(num);
 }
 public E remove(int i) {
 List<E> sync = Collections.synchronizedList(list);
 if (sync.isEmpty()) return null;
 return sync.remove(i);
 }
}

• Martin Spamer: Not really, for interview questions and production code you do not re-invent the wheel, for academic purposes it appears fine. docs.oracle.com/javase/tutorial/essential/concurrency/…

Answer 1

Definitely not!

While using the ReadWriteLock pattern instead of simple intrinsic locking for collections is a good thing to do, you are doing almost everything wrong. I recommend you to take a deeper look at Synchronizers in Java and also the Decorator Pattern. Following points are done wrong in your snippet:

• You are having a ReadWriteLock attribute but don't use it.

• The synchronized Collection Wrapper is used in some methods but doing that will not create any kind of Thread-Safety since the Mutex aquired is chaning every time the method is invoked.

• You are having two other locks where as one should be the read and the other one the write lock. But using those to guard code sections in useless, since they both have no connection to each other and you don't even lock every critical section.

Important for you to understand is, that when using locking every access must be serialized / guarded or your locking is worthless. Before you are having an important interview you may take a look at the Book Concurrency In Practice which gives a very good introduction to Java Concurrency.