Threads: Avoid synchronized(this) in Java

I was thinking to improve my threading fundamentals, so I planned to make few notes for myself that will assist me in future (Obviously at the time of Interviews). Following questions and answers have been taken from stackoverflow.com. This post is a sticky notes post for me. Please do not use the answers for reference purpose.

This is a quite an interesting question, and questioner has followed every answer quite religiously. Though question seems like naïve, questioner has taken the discussion to different planet altogether.

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:

• some evil code may steal your lock (very popular this one, also has an "accidentally" variant)

Try to avoid synchronizing on this because that would allow everybody from the outside who had a reference to that object to block my synchronization. Instead, I create a local synchronization object:

public class Foo {

    private final Object syncObject = new Object();

    …

}

Now I can use that object for synchronization without fear of anybody “stealing” the lock.

• all synchronized methods within the same class use the exact same lock, which reduces throughput

The issue is that a synchronized method is actually just syntax sugar for getting the lock on this and holding it for the duration of the method. Thus, public synchronized void setInstanceVar() would be equivalent to something like this:

public void setInstanceVar() {

    synchronized(this) {

        instanceVar++;

    }

}

This is bad for two reasons:

• All synchronized methods within the same class use the exact same lock, which reduces throughput
• Anyone can get access to the lock, including members of other classes.

• you are (unnecessarily) exposing too much information

The equivalent of that is:

synchronized (this)

{

}

(And no, you shouldn't generally do it in either C# or Java. Prefer locking on private references which nothing else has access to. You may be aware of that already, of course - but I didn't want to leave an answer without the warning :)

People 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.

Answer

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.

But in the same thread one more person has proved that defensive approach is required in some of the cases while dealing with threads.

Let's say your system is a servlet container, and the object we're considering is the ServletContextimplementation. Its getAttribute method must be thread-safe, as context attributes are shared data; so you declare it as synchronized. Let's also imagine that you provide a public hosting service based on your container implementation.

I'm your customer and deploy my "good" servlet on your site. It happens that my code contains a call togetAttribute.

A hacker, disguised as another customer, deploys his malicious servlet on your site. It contains the following code in the init method:

synchronized (this.getServletConfig().getServletContext()) {

   while (true) {}

}

Assuming we share the same servlet context (allowed by the spec as long as the two servlets are on the same virtual host), my call on getAttribute is locked forever. The hacker has achieved a DoS on my servlet.

This attack is not possible if getAttribute is synchronized on a private lock, because 3rd-party code cannot acquire this lock.

I admit that the example is contrived and an over simplistic view of how a servlet container works, but IMHO it proves the point.

So I would make my design choice based on security consideration: will I have complete control over the code that has access to the instances? What would be the consequence of a thread's holding a lock on an instance indefinitely?

Discussion can be concluded saying that it is mandatory to define the purpose of threading and various other aspect of the application, and accordingly we must implements the Synchronization.