Assigning an object within a synchronized block based on that object (Java)

Question

I ran into some (production!) code that looks like the snippet below:

synchronized(some_object) {
    some_object = new some_object()
}

I would expect this to be subject to all sorts of horrible race conditions, and that a second thread Could possibly enter this block one the new object is created. My Java chops aren't good enough to state definitively the expected behavior of above, so curious what you folks have to say before I refactor this.

Answer 1

This could actually be OK depending on what is going on. You would need to understand the larger context. The synchronization will be on the object that's pointed to by some_object at the beginning of the block. There is not enough information from your description to see that it's a bug.

The synchronization itself will work just fine.

Answer 2

As Francis says, this may not be a problem. Your snippet is equivalent to this:

SomeObject saved = some_object;
synchronized(saved) {
  some_object = new SomeObject()
}

Answer 3

The synchronization is on the object that was referenced when entering the synchronized block. Pointing the reference to another object inside the synchronized block does not effect the synchronization at all. It is still synchronized over the "old" object.

Answer 4

This is pretty bad. synchronized is best used on final class members.

The modern approach to creating an object in a thread safe manner is using AtomicReference compareAndSet in a loop as discussed in Goetz's Java Concurrency in Action (chap 15). This does not block your threads and offers far greater performance than a synchronized block.

private final AtomicReference<SomeObject> someObject = new AtomicReference<>();


void buildIt() {
   SomeObject obj = new SomeObject();
   SomeObject current = someObject.get();  //probably null, but doesn't matter
   while (true) {
      if (someObject.compareAndSet(current, obj)) 
          break;
   }
}