Should you synchronize the run method? Why or why not?

Question

I have always thought that synchronizing the run method in a java class which implements Runnable is redundant. I am trying to figure out why people do this:

public class ThreadedClass implements Runnable{
    //other stuff
    public synchronized void run(){
        while(true)
             //do some stuff in a thread
        }
    }
}

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?

Solution 1

Synchronizing the run() method of a Runnable is completely pointless unless you want to share the Runnable among multiple threads and you want to sequentialize 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 21+ years of Java.

Solution 2

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

public class ThreadedClass implements Runnable{
    private final Object lock = new Object();

    public void run(){
        synchronized(lock) {
            while(true)
                 //do some stuff in a thread
            }
        }
    }
}

Edit in response to comment:

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.

class Stack {
    int[] data = new int[10];
    int pos = 0;

    void add(int inc) {
        if(pos == data.length) {
            int[] tmp = new int[pos*2];
            for(int i = 0; i < pos; i++) tmp[i] = data[i];
            data = tmp;
        }
        data[pos++] = inc;
    }

    int remove() {
        return data[pos--];
    }

    void makeExactSize(int size) {
        int[] tmp = new int[size];
        for(int i = 0; i < size; i++) tmp[i] = data[i];
        data = tmp;
    }
}

Solution 3

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.

Solution 4

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.

View more solutions

Comments

[MHP]

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--

[corsiKa]

The answer to that is longer than a comment. I will edit it in the answer.

[ratchet freak]

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)

[MHP]

"...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.)

[Voo]

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)

[Voo]

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.

[ratchet freak]

@voo I meant that calling t.doThis();t.doThat(); when they are both declared synchronized allows the JVM to keep holding the lock between calling this and that

[corsiKa]

@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.

[Voo]

@ratchet freak Yeah I understood you. But at least if the methods are inlined this could be done anyhow and then I've never stumbled upon a situation where I wanted to make the thread synchronize on its own instance. Usually you want several threads to synchronize on some higher level structure (ie in glowcoder's example the stack would hardly implement run(), but some threads would operate on it and those would need the synchronization). Maybe I'm missing some usual scenario, but I can't imagine one.

[Voo]

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.

[user207421]

Doesn't answer the question. The question is specifically about the 'run()' method, which has a special relationship with the calling thread.

[user207421]

@KenyakornKetsombut Wrong how exactly? You have just encountered what other example? Evidence please, not just anecdote. This is science, among other things.

[Elia12345]

same benefits with lock as in the answer are described here: docs.oracle.com/javase/tutorial/essential/concurrency/…

[Benoit Duffez]

What if the Runnable has to read a shared object? I just had a NPE in a block with if (sharedObject != null) { threadObject = sharedObject.getField(); } because sharedObject was nullified by another thread between the two instructions. Would it be stupid to declare my run method synchronized? This wouldn't be in contradiction of the purpose (running in a background thread), it just would block writes to the shared object when the thread is running.

[user207421]

@BenoitDuffez Then the code should synchronize on the shared object while it is being accessed. Not on the Runnable for its entire duration.

[leonidos79]

@user207421 While this a valid remark, I would add that "the code should synchronize on special-purpose lock object - a private field declared next to run() method, for example", as the shared object may be nullified by another process or task as @BenoitDuffez pointed out, and you cannot use synchronized() on something that may be null.