Confusing use of synchronized in Java: pattern or anti-pattern?

Question

I'm doing a code review for a change in a Java product I don't own. I'm not a Java expert, but I strongly suspect that this is pointless and indicates a fundamental misunderstanding of how synchronization works.

synchronized (this) {
    this.notify();
}

But I could be wrong, since Java is not my primary playground. Perhaps there is a reason this is done. If you can enlighten me as to what the developer was thinking, I would appreciate it.

Answer 1

It certainly is not pointless, you can have another thread that has a reference to the object containing the above code doing

synchronized(foo) {
    foo.wait();
}

in order to be woken up when something happens. Though, in many cases it's considered good practice to synchronize on an internal/private lock object instead of this.

However, only doing a .notify() within the synchronization block could be quite wrong - you usually have some work to do and notify when it's done, which in normal cases also needs to be done atomically in regards to other threads. We'd have to see more code to determine whether it really is wrong.