Badger Badger - 4 months ago 11
Java Question

Is it fine to call this synchronized method from a synchronized block?

Simply put, I'm wondering if this changes the behavior. I'm assuming yes, because calling someMethod() will lock the entire object, instead of just the list object? But I'm still new to this synchronization stuff, so I'd like some more educated feedback.

The before:

public void run() {
int i = 0;

while (!end) {
synchronized (list) {
while (list.size() == i) {
try {
list.wait();
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
}
}


The After:

public void run() {
int i = 0;

while (!end) {
synchronized (list) {
while (list.size() == i) {
someMethod();
}
}
}
}

public synchronized void someMethod() {
try {
list.wait();
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}

Answer

You are correct - the new code has got different semantics, as someMethod() is indeed synchronized on the instance it is being invoked on (and as such, that synchronization is entirely unrelated to the one on list). However, the call to someMethod() will take place while the monitor on list is being held, so a call to run() is "equally thread safe" with respect to list.

On the other hand, you have now introduced the possibility for multiple threads to call someMethod() directly at the same time. You have also introduced a (probably unnecessary) potential for deadlock with other threads due to the extra synchronization on the object itself. I would recommend this instead:

public void someMethod() {
    synchronized (list) {
        try {
            list.wait();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

This method is now safe both for individual use and for being called through run() - note that it is safe to synchronize on an object that you have already synchronized on; the thread won't block itself.

Comments