Why the lock wrong?

Question

public class ListHelper<E> {
  public List<E> list =
  Collections.synchronizedList(new ArrayList<E>());
  ...
  public synchronized boolean putIfAbsent(E x) {
  boolean absent = !list.contains(x);
  if (absent)
   list.add(x);
   return absent;
  }
}

I can not understand why this does not work.

If I change the list to private field, isn't this code correct?

I have no clue what you want to achive there and why whatever doesn´t work, but it looks like you´re immitating a `Set` and might want to use it over a `List`. — SomeJavaGuy, Dec 15 '16 at 12:36

score 1 · Answer 1 · answered Dec 15 '16 at 12:37

The code does not work (i.e. does not reliably synchronize access), because other code can directly access the list, bypassing your synchronized lock.

Making it private would prevent that.

The fact that you are using a synchronizedList does not help (it would, if the synchronization lock involved in that would be the same as the one your method is using, but it is not).

If you want to keep the list public, you could update your method to synchronize on the same lock (i.e. the list itself).

This approach is detailed in the JavaDoc for synchronizedList.

Why the lock wrong?

1 Answers1