CODEWITHSUNDEEP
javamultithreadingasynchronous
Badger
Badger

Reputation: 301

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();
    }
}

Upvotes: 0

Views: 73

Answers (2)

Aasmund Eldhuset
Aasmund Eldhuset

Reputation: 37960

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.

Upvotes: 2

ParkerHalo
ParkerHalo

Reputation: 4430

With syncronized you don't really 'lock' an object. You will just make sure that access from everyone who is synchronizing on that specific object is 'regulated' with a lock.

A synchronized method is synchronizing on this.

That means, if you enter a syncronized block on some object list you can call a syncronized method without a problem at first

Some things to be thought about:

This code doesn't produce a deadlock:

public void foo() {
    synchronized (this) {
        someMethod();
    }
}

public synchronized void someMethod() {
    // ...
}

because you already have the "lock" on this, but this:

public void foo() {
    synchronized (list) {
        someMethod();
    }
}

public synchronized void someMethod() {
    // ...
}

may produce a deadlock with some other thread! You'll have to be really careful if you enter a synchronized section within another synchronized section

Upvotes: 2

Related Questions