Any reason to synchronize a run method?

[James Davison]

I ran across the following code which got me to thinking: Is there any good reason to synchronize a run method for a class that implements Runnable? Because this method implements a while(true) statement in the method body, a second invocation of the run method will never be executed. Perhaps that was the original developer's intention?

    
/* (non-Javadoc)
 * @see java.lang.Runnable#run()
 */
public synchronized void run() {

The same class contains the following method. Ordinarily, I would synchronize a method like this too, if I used an ArrayList. However, purgeQueue is Vector. For that reason, I would not think it necessary to synchronize the method itself (although I know it does not really hurt). Am I correct?

public final synchronized void addDocument(String docPath) {
Date curTimeStamp = new Date();
Document aDoc = new Document(docPath,curTimeStamp);
this.purgeQueue.add(aDoc);
}

[ April 10, 2007: Message edited by: James Davison ]

[Chris Beckey]

They could have been trying to protect against running multiple threads on the same instance (like the following example). It might well be legitimate but it first impression is that of a shotgun approach to a threading bug.

public class MultipleThreadsOneRunnable
implements Runnable
{
public synchronized void run()
{
System.out.println(Thread.currentThread().getName() +  " is starting.");

for(int n=0; n<10; ++n)
{
try
{
Thread.sleep(1000);
System.out.println(Thread.currentThread().getName() +  " running.");
} 
catch (InterruptedException x)
{
x.printStackTrace();
}
}

System.out.println(Thread.currentThread().getName() +  " is finishing.");
}
 
/**
 * @param args
 */
public static void main(String[] args)
{
MultipleThreadsOneRunnable runnable = new MultipleThreadsOneRunnable();

Thread threadOne = new Thread(runnable);
Thread threadTwo = new Thread(runnable);

threadOne.start();
threadTwo.start();
}
 
}

[Rahul Bhattacharjee]

If you make run menthod syncronized then the lock of that class would be occupied before executing the run method.In case you start multiple threads using the same runnable implementation in the constructor of the Thread then it would work.But until the first thread finishes the second thread cannot start and until the second thread finishes the third cannot start as all the threads depends on lock of a single object.

[Nitesh Kant]

As chris correctly said, its a shotgun approach of making a multi-threaded code a single threaded one
In effect, if you synchronize the run method, there are two outcomes:
1) Synchronize does not help, if the threads are created using different runnable instances.
2) There is no multi-threading, if all threads are created using the same runnable. Its as good as directly calling the run method from the same thread of execution
[ April 11, 2007: Message edited by: Nitesh Kant ]

[Ilja Preuss]

Originally posted by Nitesh Kant:

2) There is no multi-threading, if all threads are created using the same runnable. Its as good as directly calling the run method from the same thread of execution

Not exactly: the thread that started these threads will still be able to run in parallel. So it works like a working queue or something?

[Ilja Preuss]

Originally posted by Rahul Bhattacharjee:
If you make run menthod syncronized then the lock of that class would be occupied before executing the run method.In case you start multiple threads using the same runnable implementation in the constructor of the Thread then it would work.But until the first thread finishes the second thread cannot start and until the second thread finishes the third cannot start as all the threads depends on lock of a single object.

Well, it could also happen that the third thread is executed before the second.

[Nitesh Kant]

Originally posted by Ilja Preuss:


Not exactly: the thread that started these threads will still be able to run in parallel. So it works like a working queue or something?

Yeah, that is true, it behaves like a work queue processed by a single thread.
[ April 12, 2007: Message edited by: Nitesh Kant ]