Title: Confusing use of synchronized in Java: pattern or anti-pattern?
Tags: java;multithreading;synchronization;synchronized
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.
Comment: @BalusC, looks like stackexchange is getting redundant with stackoverflow,programmers and codereview

Comment: I *think* this is more appropriate for http://codereview.stackexchange.com

Comment: @ringbearer: [there are 65 already](http://stackexchange.com/sites). I agree to a certain degree that SO isn't really the right place for code reviews as they may be subjective and too localized. SO is more for real concrete programming questions which can have only 1 real answer. Programmers is more for subjective but constructive programmer's questions which can have different valid answers.

Comment: Ah, was not aware of codereview.  Thanks.

Comment: Thanks for the link claymore.  It provides more context than I had even thought of.

Comment: You can get rid of the `this` in front of the `notify()` method invocation. It doesn't do or signify anything and just adds to the clutter.

Comment: "pattern" I think you mean "idiom".

Comment: Are you wondering if the synchronization is necessary before the call to `notify()`?  The thread **must** be holding the object's monitor when it calls `notify()`, so this is required in general.

Comment: Reading material:
http://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java

Here is the accepted answer: 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.
Comment for this answer: Seems obvious in retrospect.  Now I need to go untangle how this all works.
Comment for this answer: Well, I can't find where he calls this.wait(), so I've sent feedback asking if this is really beckcarolyn@example.net.

Here is another answer: The Java API documentation for Object.notify() states that the method "should only be called by a thread that is the owner of this object's monitor".  So the use could be legitimate depending upon the surrounding context.

Here is another answer: If that is all that is in the synchonized block then it is an antipattern, the point of synchronizing is to do something within the block, setting some condition, then call notify or notifyAll to wake up one or more waiting threads.
When you use wait and notify you have to use a condition variable, see this Oracle tutorial:
Note: Always invoke wait inside a loop that tests for the condition being waited for. Don't assume that the interrupt was for the particular condition you were waiting for, or that the condition is still true.
You shouldn't assume you received a notification just because a thread exited from a call to Object#wait, for multiple reasons:
When calling the version of wait that takes a timeout value there's no way to know whether wait ended due to receiving a notification or due to timing out.
You have to allow for the possibility that a Thread can wake up from waiting without having received a notification (the "spurious wakeup").
The waiting thread that receives a notification still has to reacquire the lock it gave up when it started waiting, there is no atomic linking of these two events; in the interval between being notified and reacquiring the lock another thread can act and possibly change the state of the system so that the notification is now invalid.
You can have a case where the notifying thread acts before any thread is waiting so that the notification has no effect. Assuming one thread will enter a wait before the other thread will notify is dangerous, if you're wrong the waiting thread will hang indefinitely.
So a notification by itself is not good enough, you end up guessing about whether a notification happened when the wait/notify API doesn't give you enough information to know what's going on. Even if other work the notifying thread is doing doesn't require synchronization, updating the condition variable does; there should at least be an update of the shared condition variable in the synchronized block.
Comment for this answer: This is what I thought, but it seems that sometimes you may want to synchronize the synchronization.
Comment for this answer: That's definitely not always true.  Maybe the thing being done before the notify does not need to be synchronized.  There is no harm in granular synchronization, that is typically a good thing.  A simple example might be a background thread that is waiting on the user to click a button (via waiting on an object).  When Swing notifies the ActionListener, it could perform the notify using the code above.
Comment for this answer: @Mark sounds very dangerous if condition is updated without guard; the other thread examining the condition can see inconsistent state.

Here is another answer: This is generally not a anti-pattern, if you still want to use intrinsic locks. Some may regard this as an anti pattern, as the new explicit locks from java.util.concurrent are more fine grained.
But your code is still valid. For instance, such code can be found in a blocking queue, when an blocking operation has succeeded and another waiting thread should be notified. Note however that concurrency issues are highly dependent on the usage and the surrounding code, so your simple snippet is not that meaningful.

Here is another answer: This is perfectly fine.  According to the Java 6 Object#notify() api documentation:
This method should only be called by a thread that is the owner of this object's monitor.
Comment for this answer: "Fine" as in a correct use of the API. But the OP was asking something whetehr it was a *pattern* or *antipattern*.

{'source': 'StackExchangeDataset', 'subset': 'stackoverflow.com', 'metadata': {'AnswerCount': 5, 'CommentCount': 9, 'ContentLicense': 'CC BY-SA 3.0', 'CreationDate': '2011:00:09/30/11', 'FavoriteCount': 2.0, 'PostTypeId': 1, 'Tags': '<java><multithreading><synchronization><synchronized>', 'Title': 'Confusing use of synchronized in Java: pattern or anti-pattern?', 'ViewCount': 1206, 'id': 7613027, 'user_AboutMe': "<p>I'm just this guy, you know?</p>\n", 'user_AccountId': 28129.0, 'user_DisplayName': 'i_am_jorf', 'user_Reputation': 52787.0, 'user_Views': 6211.0, 'user_id': 74815.0}}