Java: Is it common practice to create a new Thread when a lock is held?

Question

Hi all,

I've got a question related but not identical to my first question ever here:

http://stackoverflow.com/questions/2340114/java-what-happens-when-a-new-thread-is-started-from-a-synchronized-block

Is it a common practice to create and start() a new Thread when you're holding a lock?

Would that be a code smell?

Basically I've got a choice between doing this:

public synchronized void somemethod() {

    (every time I find a callback to be notified I start a thread)   
    Thread t = new Thread( new Runnable() {
       void run() {
           notifySomeCallback();      
       }
    }
    t.start();
    ... 
    (lengthy stuff performed here, keeping the lock held)
    ...
}

or this:

public void somemethod() {
    (create a list of callbacks to be notified)
    synchronized(this){
      (potentially add callbacks)       
      ... 
      (lengthy stuff performed here, keeping the lock held)
      ...
    }
    (notify the callbacks without holding a lock and once
     we know the lock has been released)
}

I think the latter is better but I wanted to know if there are cases where the first option would be ok? Do you sometimes do that? Have you seen it done?

Answer 1

It depends on whether or not you want the callbacks to be executed concurrently with the lengthy stuff or not. If we are talking about a Swing GUI, option 1 is not good, because you shouldn't do Swing operations in several concurrent threads, so I propose the following:

public void somemethod() {   
   Thread t = new Thread( new Runnable() {
       void run() {
           synchronized(this) {
             doLengthyStuff();
           }
       }
    }
    t.start();

    (notify the callbacks)
}

Answer 2

answer3:

• You should always hold on to a lock as short as possible. So only the resource which is potentially referenced to from multiple threads should be locked for the smallest amount of time when the chance of a 'corrupt' resource exists (e.g. the writer thread is updating the resource)

• Don't spin off a thread for every little thing which needs to be done. In the case of your callback threads, have 1 callback thread work off a queue of things to do.

Answer 3

You are aware that the two code snippets will result in different execution orders. The first one will run the callbacks asynchronously, while the lengthy stuff is being performed. The second one will finish doing the lengthy stuff first and then call the callbacks.

Which one is better depends on what the callbacks need to do. It might well be a problem if they need lengthy stuff to be done first.

Who is waiting on the lock? If the callbacks need the lock to run, it makes little sense to fire them, while you still hold the lock. All they would do is just wait for lengthy stuff to be done anyway.

Also, in the first snippet, you have one thread per callback. The second snippet is not explicit, but if you have only one thread for all of them, this is another difference (whether the callbacks run simultaneously or in sequence). If they all need the same lock, you might as well run them in sequence.

If you want to run many callbacks with one or more threads, consider using an Executor instead of managing the threads yourself. Makes it very easy to configure an appropriate number of threads.