Is this ok? Synchronized( thread ), then thread=null in the synch block

I see this:

// thread is a member of this class

synchronized( this.thread )
{
  this.thread.running = false;
  this.thread.notifyAll(); // Wake up anything that was .waiting() on
  // the thread
  this.thread = null;  // kill this 开发者_开发问答thread reference.
  // can you do that in a synchronized block?
}

Is it ok to set the thread=null while still keeping a lock on it?

I found this nugget in a bit of BB code.

Yes, that's fine. The synchronized statement will take a copy of the reference that it's locking on, and use the copy to work out what to unlock at the end.

Section 14.19 of the Java Language Specification isn't actually clear about this, but it does state that the expression is evaluated at the start - and doesn't mention evaluating it again later on.

There's a difference:

synchronized( this.thread )

You are synchronizing on the Object the field this.thread points to

this.thread = null;

You are reassigning the field. You are not doing anything with the object you referenced above, so the lock is still valid.

The synchronized expression is dereferenced on entry, so any later users of this lock will get a NullPointerException. You can work around that by putting a null check ahead of the synchronized block, but then you've introduced a race condition.

You can do it, but it's almost certain that the code is wrong for whatever it is trying to achieve. Post the entire code, and I guarantee that it is obviously the programmer doesn't understand concurrency.

Do not reassign a variable which is used for synchronization.

You only have a problem if you also have a block which assigns a new value to thread. In that case you have a race condition as the two blocks don't lock on the same object, but will update the same field and it will be random as to which block assigns the value last.