eljenso eljenso - 6 months ago 17
Java Question

Avoid synchronized(this) in Java?

Whenever a question pops up on SO about Java synchronization, some people are very eager to point out that

synchronized(this)
should be avoided. Instead, they claim, a lock on a private reference is to be preferred.

Some of the given reasons are:



Other people, including me, argue that
synchronized(this)
is an idiom that is used a lot (also in Java libraries), is safe and well understood. It should not be avoided because you have a bug and you don't have a clue of what is going on in your multithreaded program. In other words: if it is applicable, then use it.

I am interested in seeing some real-world examples (no foobar stuff) where avoiding a lock on
this
is preferable when
synchronized(this)
would also do the job.

Therefore: should you always avoid
synchronized(this)
and replace it with a lock on a private reference?





Some further info (updated as answers are given):


  • we are talking about instance synchronization

  • both implicit (synchronized methods) and explicit form of
    synchronized(this)
    are considered

  • if you quote Bloch or other authorities on the subject, don't leave out the parts you don't like (e.g. Effective Java, item on Thread Safety: "Typically it is the lock on the instance itself, but there are exceptions.")

  • if you need granularity in your locking other than
    synchronized(this)
    provides, then
    synchronized(this)
    is not applicable so that's not the issue


Answer

I'll cover each point separately.

  1. Some evil code may steal your lock (very popular this one, also has an "accidentally" variant)

    I'm more worried about accidentally. What it amounts to is that this use of this is part of your class' exposed interface, and should be documented. Sometimes the ability of other code to use your lock is desired. This is true of things like Collections.synchronizedMap (see the javadoc).

  2. All synchronized methods within the same class use the exact same lock, which reduces throughput

    This is overly simplistic thinking; just getting rid of synchronized(this) won't solve the problem. Proper synchronization for throughput will take more thought.

  3. You are (unnecessarily) exposing too much information

    This is a variant of #1. Use of synchronized(this) is part of your interface. If you don't want/need this exposed, don't do it.