Is this a bug in the Java SynchronizedCollection class?

Question

There is an inner class SynchronizedCollection - inside java.util.Collections with two constructors. the first takes a collection, and the other takes a collection and a mutex. the former constructor checks the argument for being non-zero. but later do not !. here is the implementation.

SynchronizedCollection(Collection<E> c) { if (c==null) throw new NullPointerException(); this.c = c; mutex = this; } SynchronizedCollection(Collection<E> c, Object mutex) { this.c = c; this.mutex = mutex; } 

with this implementation, I can break the class invariant by sending a zero per second constructor.

I believe this should be something like this:

  SynchronizedCollection(Collection<E> c) { this(c,this) } SynchronizedCollection(Collection<E> c, Object mutex) { if (c==null) throw new NullPointerException(); this.c = c; this.mutex = mutex; } 

however, I cannot convince myself that Josh Bloch and Neil Gufter could not see it. so can you really tell me what i missed here?

---

edited: possible attack

  Map<String, String> m = new Map<String, String>(){ @Override public int size() { // TODO Auto-generated method stub return 0; } . . . @Override public Collection<String> values() { return null; } }; Map<String, String> synchronizedMap = Collections.synchronizedMap(m); Collection<String> values = synchronizedMap.values(); 

Answer 1

Of course, this is a mistake. Both constructors must be consistent, or both must throw an exception, or no one should throw.

This is fixed in Java 8. Now both constructors will throw an exception:

 SynchronizedCollection(Collection<E> c) { this.c = Objects.requireNonNull(c); mutex = this; } SynchronizedCollection(Collection<E> c, Object mutex) { this.c = Objects.requireNonNull(c); this.mutex = Objects.requireNonNull(mutex); } 

Answer 2

Both of these constructors are protected by the package, and only the first can be used with the null argument using the public synchronizedList() and synchronizedSet() Collections methods.

Another constructor is used internally (in the Collections class), and the first argument can never be null in various implementations (call code), so you cannot break it.

You can always try to create something in the java.util package, but most likely get a SecurityException .