It seems redundant and unnecessary since they are obtaining the object's lock for another thread. Or rather, they are making explicit that only one thread has access to the run() method. But since its the run method, isn't it itself its own thread? Therefore, only it can access itself and it doesn't need a separate locking mechanism?

I found a suggestion online that by synchronizing the run method you could potentially create a de-facto thread queue for instance by doing this:

public void createThreadQueue(){
ThreadedClass a = new ThreadedClass();
new Thread(a, "First one").start();
new Thread(a, "Second one, waiting on the first one").start();
new Thread(a, "Third one, waiting on the other two...").start();
}

I would never do that personally, but it lends to the question of why anyone would synchronize the run method. Any ideas why or why not one should synchronize the run method?

the queue is faulty (the Object monitor is not fair, and the second thread might get to run before the first), the only reason I can imagine is to ensure that when a runnable is submitted twice to a executor/thread it doesn't create races
–
ratchet freakSep 6 '11 at 0:18

@irreputable My professor did it in an example. I would never personally--except I am waiting to see if there is any brilliant reason for doing it that no one hasn't been pointed out yet.
–
MHPSep 6 '11 at 0:20

@ratchet Good point. I guess you would only want the synchronized run it if there is a strange reason why another Thread might be executed on the same object. But even then, I would solve it differently I think.
–
MHPSep 6 '11 at 0:23

1

@MHP an atomic boolean hasRun and a if(!hasRun.CompareAndSwap(false,true))return; in the run is better (as it doesn't block a thread and ensures the run is only executed once) but requires extra code and a separate var
–
ratchet freakSep 6 '11 at 0:29

6 Answers
6

Syncrhonizing the run() method of a Runnable is completely pointless unless you want to share the Runnable among multiple threads and you want to serialize the execution of those threads. Which is basically a contradiction in terms.

There is in theory another much more complicated scenario in which you might want to synchronize the run() method, which again involves sharing the Runnable among multiple threads but also makes use of wait() and notify(). I've never encountered it in 14+ years of Java.

Why? Minimal extra safety and I don't see any plausible scenario where it would make a difference.

Why not? It's not standard. If you are coding as part of a team, when some other member sees your synchronized run he'll probably waste 30 minutes trying to figure out what is so special either with your run or with the framework you are using to run the Runnable's.

There is 1 advantage to using synchronized void blah() over void blah() { synchronized(this) { and that is your resulting bytecode will be 1 byte shorter, since the synchronization will be part of the method signature instead of an operation by itself. This may influence the chance to inline the method by the JIT compiler. Other than that there is no difference.

The best option is to use an internal private final Object lock = new Object() to prevent someone from potentially locking your monitor. It achieves the same result without the downside of the evil outside locking. You do have that extra byte, but it rarely makes a difference.

So I would say no, don't use the synchronized keyword in the signature. Instead, use something like

Consider what synchronization does: it prevents other threads from entering the same code block. So imagine you have a class like the one below. Let's say the current size is 10. Someone tries to perform an add and it forces a resize of the backing array. While they're in the middle of resizing the array, someone calls a makeExactSize(5) on a different thread. Now all of a sudden you're trying to access data[6] and it bombs out on you. Synchronization is supposed to prevent that from happening. In multithreaded programs you need simply NEED synchronization.

I like the enthusiasm with the byte code reference, but I really mean is there any benefit to synchronizing the method at all. My professor always writes "synchronized void run()" and I have never done that nor needed to. But that byte code bit is interesting--
–
MHPSep 5 '11 at 23:57

The answer to that is longer than a comment. I will edit it in the answer.
–
corsiKaSep 5 '11 at 23:59

actually the synchronized in the signiture means that the JIT/JVM can keep holding the lock when calling several synchronized methods (i.e. the JVM doesn't release the lock (and immediately reacquires it) when the next operation is calling the next synchronized method)
–
ratchet freakSep 5 '11 at 23:59

Ok that's true - reacquiring an already held lock should be cheaper - but then I somehow doubt that's noticeable in most situations and I've yet to see a situation where I'd wanted a synchronized method on a instance of the thread (you usually have higher order data structures for communication imo)
–
VooSep 6 '11 at 0:03

1

@ratchet I was under the impression the JIT was capable that optimization even for locks declared in the method, and for locks privately held (as opposed to the calling object's monitor). I could be mistaken as I'm unable to immediately produce a source for it.
–
corsiKaSep 6 '11 at 0:11

From my experience, it's not useful to add "synchronized" keyword to run() method. If we need synchronize multiple threads, or we need a thread-safe queue, we can use more appropriate components, such as ConcurrentLinkedQueue.

Well you could theoretically call the run method itself without problem (after all it is public). But that doesn't mean one should do it. So basically there's no reason to do this, apart from adding negligible overhead to the thread calling run(). Well except if you use the instance multiple times calling new Thread - although I'm a) not sure that's legal with the threading API and b) seems completely useless.

Also your createThreadQueue doesn't work. synchronized on a non-static method synchronizes on the instance object (ie this), so all three threads will run in parallel.

"...so all three threads will run in sequence." Thats sort of what I meant be a "de-facto" thread queue... they would just run after the prior one finished. Or did I misunderstand you? (And like I said, if i needed something like that I would never code it like that... this is just speculation.)
–
MHPSep 6 '11 at 0:02

No I just got confused. This should say "run in parallel" or something like that. I'll fix it. Basically if the run method contained only a print() statement you could every possible combination of the three sentences.
–
VooSep 6 '11 at 0:04

1

Uh actually misread the example, so yes that'd work - somewhat strange construct though. You could get exactly the same result with just a simple for(int i = 0; i < N; i++) a.run(); without the overhead.
–
VooSep 6 '11 at 0:27