In Java critical sections, what should I synchronize on?
Asked Answered
M

11

78

In Java, the idiomatic way to declare critical sections in the code is the following:

private void doSomething() {
  // thread-safe code
  synchronized(this) {
    // thread-unsafe code
  }
  // thread-safe code
}

Almost all blocks synchronize on this, but is there a particular reason for this? Are there other possibilities? Are there any best practices on what object to synchronize on? (such as private instances of Object?)

Maffick answered 6/1, 2009 at 11:31 Comment(0)
P
54

First, note that the following code snippets are identical.

public void foo() {
    synchronized (this) {
        // do something thread-safe
    }
}

and:

public synchronized void foo() {
    // do something thread-safe
}

do exactly the same thing. No preference for either one of them except for code readability and style.

When you do synchronize methods or blocks of code, it's important to know why you are doing such a thing, and what object exactly you are locking, and for what purpose.

Also note that there are situations in which you will want to client-side synchronize blocks of code in which the monitor you are asking for (i.e. the synchronized object) is not necessarily this, like in this example :

Vector v = getSomeGlobalVector();
synchronized (v) {
    // some thread-safe operation on the vector
}

I suggest you get more knowledge about concurrent programming, it will serve you a great deal once you know exactly what's happening behind the scenes. You should check out Concurrent Programming in Java, a great book on the subject. If you want a quick dive-in to the subject, check out Java Concurrency @ Sun

Prosthodontist answered 6/1, 2009 at 11:47 Comment(11)
The code snippets LOGICALLY do the same thing, but they compile to different bytecode!!Unavailing
Being very lazy and not trying it myself, you might want to explain the difference. Also, "different" is not really an issue here. If they are "equivalent", then everybody's happy.Psilomelane
They are not equivalent. See cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.htmlGossipmonger
@jason I never stated they were equivalent, although I admit that I thought they were. Still, I fail to see what DCL has to do with all this. Can you point out the relevant portion of the document. Still being lazy :)Psilomelane
@Jason - can you please point to the exact location explaining your statement? I don't see the connection in the link you posted, sorry.Prosthodontist
The double-checked locking code uses explicit "synchronized (this)" because it needs to do extra stuff before or after the sync. If you have nothing to do before or after, then they're really the same.Spermic
DCL is relevant in that it points out that the compiler and/or JMM may do unexpected things. The two code snippets, as posted, do (essentially) the same thing, albeit with different bytecode. Naively adding code before or after the synchronized block (as in the OP) may not be thread-safe.Gossipmonger
@jason Pointing to a document about DCL because "it points out that the compiler or JMM may do unexpected things" is overkill and not really to the point. You must specify WHY they are not equivalent. The majority of people agree that they are equivalent: stackoverflow.com/questions/417285Psilomelane
Note that a syncronized method is often not the best option, since we hold a lock the whole time the method runs. If it contains timeconsuming but thread-safe parts, and a not so time consuming thread-unsafe part, a synchronized method is very wrong.Nefarious
The most obvious difference is that a synchronized(this) block compiles to bytecode that is longer than a synchronized method. When you write a synchronized method, the compiler just puts a flag on the method and the JVM acquires the lock when it sees the flag. When you use a synchronized(this) block the compiler generates bytecode similar to a try-finally block that acquires and releases the lock and inlines it into your method.Vigilante
Another difference is that the synchronized flag on the method can be seen via reflection by other code. The presence of a synchronized block within a method cannot be detected without examining its bytecode. This may not seem important, and usually isn't, but an AOP framework might be able to omit a redundant lock if it could see that a method is already synchronized, for example.Vigilante
H
75

As earlier answerers have noted, it is best practice to synchronize on an object of limited scope (in other words, pick the most restrictive scope you can get away with, and use that.) In particular, synchronizing on this is a bad idea, unless you intend to allow the users of your class to gain the lock.

A particularly ugly case arises, though, if you choose to synchronize on a java.lang.String. Strings can be (and in practice almost always are) interned. That means that each string of equal content - in the ENTIRE JVM - turns out to be the same string behind the scenes. That means that if you synchronize on any String, another (completely disparate) code section that also locks on a String with the same content, will actually lock your code as well.

I was once troubleshooting a deadlock in a production system and (very painfully) tracked the deadlock to two completely disparate open source packages that each synchronized on an instance of String whose contents were both "LOCK".

Handfasting answered 13/1, 2009 at 16:12 Comment(2)
+1 for the helpful real-world anecdote about the String instance lock.Whoreson
Strings are interned only if they are purposely interned by intern() (exotic case) or they are written in codeUriia
P
54

First, note that the following code snippets are identical.

public void foo() {
    synchronized (this) {
        // do something thread-safe
    }
}

and:

public synchronized void foo() {
    // do something thread-safe
}

do exactly the same thing. No preference for either one of them except for code readability and style.

When you do synchronize methods or blocks of code, it's important to know why you are doing such a thing, and what object exactly you are locking, and for what purpose.

Also note that there are situations in which you will want to client-side synchronize blocks of code in which the monitor you are asking for (i.e. the synchronized object) is not necessarily this, like in this example :

Vector v = getSomeGlobalVector();
synchronized (v) {
    // some thread-safe operation on the vector
}

I suggest you get more knowledge about concurrent programming, it will serve you a great deal once you know exactly what's happening behind the scenes. You should check out Concurrent Programming in Java, a great book on the subject. If you want a quick dive-in to the subject, check out Java Concurrency @ Sun

Prosthodontist answered 6/1, 2009 at 11:47 Comment(11)
The code snippets LOGICALLY do the same thing, but they compile to different bytecode!!Unavailing
Being very lazy and not trying it myself, you might want to explain the difference. Also, "different" is not really an issue here. If they are "equivalent", then everybody's happy.Psilomelane
They are not equivalent. See cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.htmlGossipmonger
@jason I never stated they were equivalent, although I admit that I thought they were. Still, I fail to see what DCL has to do with all this. Can you point out the relevant portion of the document. Still being lazy :)Psilomelane
@Jason - can you please point to the exact location explaining your statement? I don't see the connection in the link you posted, sorry.Prosthodontist
The double-checked locking code uses explicit "synchronized (this)" because it needs to do extra stuff before or after the sync. If you have nothing to do before or after, then they're really the same.Spermic
DCL is relevant in that it points out that the compiler and/or JMM may do unexpected things. The two code snippets, as posted, do (essentially) the same thing, albeit with different bytecode. Naively adding code before or after the synchronized block (as in the OP) may not be thread-safe.Gossipmonger
@jason Pointing to a document about DCL because "it points out that the compiler or JMM may do unexpected things" is overkill and not really to the point. You must specify WHY they are not equivalent. The majority of people agree that they are equivalent: stackoverflow.com/questions/417285Psilomelane
Note that a syncronized method is often not the best option, since we hold a lock the whole time the method runs. If it contains timeconsuming but thread-safe parts, and a not so time consuming thread-unsafe part, a synchronized method is very wrong.Nefarious
The most obvious difference is that a synchronized(this) block compiles to bytecode that is longer than a synchronized method. When you write a synchronized method, the compiler just puts a flag on the method and the JVM acquires the lock when it sees the flag. When you use a synchronized(this) block the compiler generates bytecode similar to a try-finally block that acquires and releases the lock and inlines it into your method.Vigilante
Another difference is that the synchronized flag on the method can be seen via reflection by other code. The presence of a synchronized block within a method cannot be detected without examining its bytecode. This may not seem important, and usually isn't, but an AOP framework might be able to omit a redundant lock if it could see that a method is already synchronized, for example.Vigilante
P
46

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

Piteous answered 6/1, 2009 at 11:35 Comment(9)
+1 this is what I do too. Don't use strings though as String references aren't truly localized unless you're careful.Louth
And who would try to "block your synchronization" on your object(s) in such a way that synchronizing on this would become dangerous, unpractical or unusable?Psilomelane
@eljenso, who knows? Anybody with a reference might. :)Piteous
Do make stack traces more readable, you might want to give the class of the locked object a name that shows which lock it is: private static class Lock { }; private final Object lock = new Lock();.Posada
@eljenso, I think it would be less likely the result of some malicious code and more likely a bug that would cause that issueSickroom
If you have bugs in your program, then debug. I am very skeptical of the fact that avoiding synchronized(this) will help you if you already don't have a real clue of what's going on in your multi-threaded program.Psilomelane
@Psilomelane - I concur. No reasonable argument as to why to do this. If you have a greedy thread hogging your resources, solve the design problem.Prosthodontist
@Psilomelane - I concu. Locking on "this" can allow an external caller to "atomicize" multiple method calls, which can be invaluable; consider iterating a synchronized collection; or the pain it is that PrintWriters using a private lock - ever tried writing a stack trace without interruption?Belgium
Another problem with locking on 'this' blindly is that you could have several methods contending on the same lock that logically do not need to be mutually exclusive.Americanize
C
6

Just to highlight that there are also ReadWriteLocks available in Java, found as java.util.concurrent.locks.ReadWriteLock.

In most of my usage, I seperate my locking as 'for reading' and 'for updates'. If you simply use a synchronized keyword, all reads to the same method/code block will be 'queued'. Only one thread can access the block at one time.

In most cases, you never have to worry about concurrency issues if you are simply doing reading. It is when you are doing writing that you worry about concurrent updates (resulting in lost of data), or reading during a write (partial updates), that you have to worry about.

Therefore a read/write lock makes more sense to me during multi-threaded programming.

Communist answered 6/1, 2009 at 14:41 Comment(1)
ReadWriteLocks don't always have good performance.Transubstantiation
M
5

You'll want to synchronize on an object that can serve as a Mutex. If the current instance (the this reference) is suitable (not a Singleton, for instance), you may use it, as in Java any Object may serve as the Mutex.

In other occasions, you may want to share a Mutex between several classes, if instances of these classes may all need access to the same resources.

It depends a lot on the environment you're working in and the type of system you're building. In most Java EE applications I've seen, there's actually no real need for synchronization...

Mir answered 6/1, 2009 at 11:37 Comment(0)
B
4

Personally, I think the answers which insist that it is never or only rarely correct to sync on this are misguided. I think it depends on your API. If your class is a threadsafe implementation and you so document it, then you should use this. If the synchronization is not to make each instance of the class as a whole threadsafe in the invocation of it's public methods, then you should use a private internal object. Reusable library components often fall into the former category - you must think carefully before you disallow the user to wrap your API in external synchronization.

In the former case, using this allows multiple methods to be invoked in an atomic manner. One example is PrintWriter, where you may want to output multiple lines (say a stack trace to the console/logger) and guarantee they appear together - in this case the fact that it hides the sync object internally is a real pain. Another such example are the synchronized collection wrappers - there you must synchronize on the collection object itself in order to iterate; since iteration consists of multiple method invocations you cannot protect it totally internally.

In the latter case, I use a plain object:

private Object mutex=new Object();

However, having seen many JVM dumps and stack traces that say a lock is "an instance of java.lang.Object()" I have to say that using an inner class might often be more helpful, as others have suggested.

Anyway, that's my two bits worth.

Edit: One other thing, when synchronizing on this I prefer to sync the methods, and keep the methods very granular. I think it's clearer and more concise.

Belgium answered 15/1, 2009 at 7:35 Comment(3)
Locking against a java.lang.String is a BAD idea - as Strings get interned. The result is that other completely disparate code could end up (unintentionally) locking on your lock. Locking on an inner class whose name is descriptive is perfectly reasonable. +1 if that suggestion gets edited out.Handfasting
@Jared: Note that I specified new String("xxx"), not just "xxx". I am well aware of the dangers of interned strings.Belgium
There's a difference between "synchronize on the object itself" and "synchronize on a reference exposed explicitly for the purpose of synchronization". If you need multiple clients to synchronize, then expose a reference for them to use. That makes it much more obvious what's going on. Still no need to lock on "this".Sigfried
P
2

Synchronization in Java often involves synchronizing operations on the same instance. Synchronizing on this then is very idiomatic since this is a shared reference that is automatically available between different instance methods (or sections of) in a class.

Using another reference specifically for locking, by declaring and initializing a private field Object lock = new Object() for example, is something I never needed or used. I think it is only useful when you need external synchronization on two or more unsynchronized resources inside an object, although I would always try to refactor such a situation into a simpler form.

Anyway, implicit (synchronized method) or explicit synchronized(this) is used a lot, also in the Java libraries. It is a good idiom and, if applicable, should always be your first choice.

Psilomelane answered 6/1, 2009 at 12:26 Comment(0)
C
1

On what you synchronize depends on what other threads that might potentially get into conflict with this method call can synchronize.

If this is an object that is used by only one thread and we are accessing a mutable object which is shared between threads, a good candidate is to synchronize over that object - synchronizing on this has no point since another thread that modifies that shared object might not even know this, but does know that object.

On the other hand synchronizing over this makes sense if many threads call methods of this object at the same time, for instance if we are in a singleton.

Note that a syncronized method is often not the best option, since we hold a lock the whole time the method runs. If it contains timeconsuming but thread safe parts, and a not so time consuming thread-unsafe part, synchronizing over the method is very wrong.

Cleaver answered 23/6, 2010 at 13:43 Comment(0)
G
1

Almost all blocks synchronize on this, but is there a particular reason for this? Are there other possibilities?

This declaration synchronizes entire method.

private synchronized void doSomething() {

This declaration synchronized a part of code block instead of entire method.

private void doSomething() {
  // thread-safe code
  synchronized(this) {
    // thread-unsafe code
  }
  // thread-safe code
}

From oracle documentation page

making these methods synchronized has two effects:

First, it is not possible for two invocations of synchronized methods on the same object to interleave. When one thread is executing a synchronized method for an object, all other threads that invoke synchronized methods for the same object block (suspend execution) until the first thread is done with the object.

Are there other possibilities? Are there any best practices on what object to synchronize on? (such as private instances of Object?)

There are many possibilities and alternatives to synchronization. You can make your code thread safe by using high level concurrency APIs( available since JDK 1.5 release)

Lock objects
Executors
Concurrent collections
Atomic variables
ThreadLocalRandom

Refer to below SE questions for more details:

Synchronization vs Lock

Avoid synchronized(this) in Java?

Gourami answered 19/4, 2016 at 16:21 Comment(0)
A
1

the Best Practices is to create an object solely to provide the lock:

private final Object lock = new Object();

private void doSomething() {
  // thread-safe code
  synchronized(lock) {
    // thread-unsafe code
  }
  // thread-safe code
}

By doing this you are safe, that no calling code can ever deadlock your method by an unintentional synchronized(yourObject) line.

(Credits to @jared and @yuval-adam who explained this in more details above.)

My guess is that the popularity of using this in tutorials came from early Sun javadoc: https://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html

Audie answered 10/5, 2019 at 1:1 Comment(0)
S
0

Synchronization includes 3 parts: Atomicity, Visibility and Ordering

Synchronized block is very coarse level of synchronization. It enforces visibility and ordering just as what you expected. But for atomicity, it does not provide much protection. Atomicity requires global knowledge of the program rather than local knowledge. (And that makes multi-threading programming very hard)

Let's say we have a class Account having method deposit and withdraw. They are both synchronized based on a private lock like this:

class Account {
    private Object lock = new Object();

    void withdraw(int amount) {
        synchronized(lock) {
            // ...
        }
    }

    void deposit(int amount) {
        synchronized(lock) {
            // ...
        }
    }
}

Considering we need to implement a higher-level class which handles transfer, like this:

class AccountManager {
    void transfer(Account fromAcc, Account toAcc, int amount) {
        if (fromAcc.getBalance() > amount) {
            fromAcc.setBalance(fromAcc.getBalance() - amount);
            toAcc.setBalance(toAcc.getBalance + amount);
        }
    }
}

Assuming we have 2 accounts now,

Account john;
Account marry;

If the Account.deposit() and Account.withdraw() are locked with internal lock only. That will cause problem when we have 2 threads working:

// Some thread
void threadA() {
    john.withdraw(500);
}

// Another thread
void threadB() {
    accountManager.transfer(john, marry, 100);
}

Because it is possible for both threadA and threadB run at the same time. And thread B finishes the conditional check, thread A withdraws, and thread B withdraws again. This means we can withdraw $100 from John even if his account has no enough money. This will break atomicity.

You may propose that: why not adding withdraw() and deposit() to AccountManager then? But under this proposal, we need to create a multi-thread safe Map which maps from different accounts to their locks. We need to delete the lock after execution (otherwise will leak memory). And we also need to ensure no other one accesses the Account.withdraw() directly. This will introduce a lots of subtle bugs.

The correct and most idiomatic way is to expose the lock in the Account. And let the AccountManager to use the lock. But in this case, why not just use the object itself then?

class Account {
    synchronized void withdraw(int amount) {
        // ...
    }

    synchronized void deposit(int amount) {
        // ...
    }
}

class AccountManager {
    void transfer(Account fromAcc, Account toAcc, int amount) {
        // Ensure locking order to prevent deadlock
        Account firstLock = fromAcc.hashCode() < toAcc.hashCode() ? fromAcc : toAcc;
        Account secondLock = fromAcc.hashCode() < toAcc.hashCode() ? toAcc : fromAcc;

        synchronized(firstLock) {
            synchronized(secondLock) {
                if (fromAcc.getBalance() > amount) {
                    fromAcc.setBalance(fromAcc.getBalance() - amount);
                    toAcc.setBalance(toAcc.getBalance + amount);
                }
            }
        }
    }
}

To conclude in simple English, private lock does not work for slightly more complicated multi-threaded program.

Simferopol answered 7/6, 2021 at 19:29 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.