0

One of my textbook mentions that the argument of synchronized() must be this... I know it is wrong. But I heard that since synchronized(this) is safer, one should always use it. Is that true ? Thank you:)

bunnyshell
  • 253
  • 4
  • 12
  • What made you think that it is wrong? It is valid. You get lock on current object. "must be this" also not correct statement. You can have any other object there – kosa Jul 27 '12 at 13:58
  • 2
    "I know it is wrong" - this is a dangerously confident statement... – Nick Jul 27 '12 at 14:00
  • But I have seen code like synchronized on object come from arguments. an example public void method(SomeObject so) { synchronized(so) { //….. } } @thinksteep – bunnyshell Jul 27 '12 at 14:02

8 Answers8

5

No it does not have to be always this. Also it simply cannot be in the case of static methods, because there is no this.

Also it is sometimes considered wrong to synchronize to this, because then lock object is visible outside.

public class Example {
    private final Object lock = new Object();

    // does not compile, there is no 'this' in static context.
    public static void staticMethod() {
        synchronized (this) {
        }
    }

    public void method() {
        int x = 3;
        //there is risk that someone else outside our code
        //uses same lock
        synchronized (this) {

        }
        //this lock is private
        synchronized (lock) {
        }
    }
}
Mikko Maunu
  • 41,366
  • 10
  • 132
  • 135
1

One of my textbook mentions that the argument of synchronized() must be this... I know it is wrong.

That is incorrect. Either the textbook is incorrect, or you have misunderstood it. The Java language allows you to synchronize on any (non-null) object reference.

But I heard that since synchronized(this) is safer, one should always use it. Is that true?

No that is not true either. It is not safer, and you certainly shouldn't always lock on this.

In fact if you are writing a general purpose library class that needs to lock "itself", it is often a good idea to declare a private lock field; e.g.

    private final Object myLock = new Object();

... and lock that object rather than this. This eliminates the kind of problems that can occur is some external code decides to lock the library object for some reason, leading to unwanted contention, and possibly deadlocks between the library classes methods and the external code.


I suspect that the point that the textbook was trying to make is that all methods that are using primitive locks to give mutual exclusion and synchronization on a data structure must use the right object as the lock. This isn't necessarily the data structure object itself, but it does need to signify that object ... in some sense. (If you don't lock the object that signifies the data structure, you risk having one thread not excluding others while it uses / updates the data structure.)


Here is a sketch of the problem that private locks aim to avoid.

/** This class implements thread-safe getting / setting by synchronizing on 'this' */
public class IntHolder {
    private int value;
    public int getValue() {
        synchronized(this) { return value; }
    }
    public void setValue(int value)
        synchronized(this) { this.value = value; }
}

/* Somewhere else, some other code (the "external code") used a holder instance
   as the lock for some larger-scale synchronization. */
IntHolder h = ...
synchronized (h) {
    /* Do something that takes a long time ... */
}

The problem is that while the external code holds that lock on h, other threads won't be able to read or change the holder's value. If that was intended ... that is fine. However, if the thread-safety of the IntHolder type is intended to be "just an implementation detail" you now a potentially unexpected failure case.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
0

IMO you're better off using 'volatile' and 'synchronized' on fields and methods if you're unsure whether synchronized(this) is a good idea in that particular place.

to put it simply, volatile places an intrinsic mutex on a variable; synchronized places an intrinsic mutex on a function call; synchronized block places an intrinsic mutex around that piece of code locked by the parameter.

Usually you'd make a lock and lock on a Object, but sometimes you want to lock on the instance itself... though, I'd advise caution on locking with a whole class instance if you only need access to one of it's members/fields (something you could've solved using synchronized and volatile in the first place)

The real reason why you might want (or not want) to lock on this is scope of the lock, you can see the lock if you lock on this. if you init a private object, the lock is hidden away.

Shark
  • 6,513
  • 3
  • 28
  • 50
0

The argument of synchronized is simply which object the hold will be placed on. It depends on what you want to do in the synchronized block that will determine what the argument should be. Using this puts a hold on the current object.

As a personal anecdote, in the project I'm currently working on, all of my synchronized holds are on COM ports, so that there's no collisions between sending and receiving packets

gobernador
  • 5,659
  • 3
  • 32
  • 51
0

it doesn't matter which lock you use. Every java.lang.Object can act as a lock. You have to make sure that all operations that work on the same mutable state use the same lock.

so synchronized(this) is exactly as safe as

private final Object lock = new Object(); synchronized(lock)

Jonas Adler
  • 905
  • 5
  • 9
  • 1
    nope, that lock is private. locking on a instance of a class (with 'this') isn't private. – Shark Jul 27 '12 at 14:04
  • i wasn't talking about visibility, every lock in java is as safe as the other when it comes to locking – Jonas Adler Jul 27 '12 at 14:05
  • 2
    @JonasAdler: No it doesn't, since if the lock isn't private but its uses are private, it can be very dangerous to use it. – Daniel Jul 27 '12 at 14:06
  • Specifically, privately (ie without an external user knowing it) locking on a publicly available monitor, like `this`, can lead to deadlock if someone uses your object as a monitor on one thread, unaware that you're going to lock on it in another. – yshavit Jul 27 '12 at 14:10
  • Dani: that it absolutely true. I just thought bunnyshell meant that the acquisition of the lock is not 'safe': once the lock is acquired you can be sure that you have it no matter what – Jonas Adler Jul 27 '12 at 14:11
  • 1
    Whether it's "exactly as safe" depends on your definition of "safe". The locks work the same way, and can be used about the same way, and so can conceivably protect the same stuff. But accessibility is a big factor in "safety" in most cases; when random strangers can acquire your locks, you're generally asking for trouble -- cause there's nothing saying they'll ever be released. – cHao Jul 27 '12 at 14:11
  • Yup, seems i misinterpreted the question, thx for the comments! – Jonas Adler Jul 27 '12 at 14:26
0

If you declare a non-static method as synchronized:

public synchronized void doSomething()
{
    ...
}

then it is synchronized on this. So if your goal is for a synchronized block to be synchronized with synchronized non-static methods like the above, then you do need to use synchronized(this).

But you are correct: you can also write synchronized(someOtherObject), as long as you're aware that that will lock out synchronized methods of someOtherObject rather than of this.

(For static methods, by the way, synchronized synchronizes on the Class instance representing the containing class.)

ruakh
  • 175,680
  • 26
  • 273
  • 307
0

No , this is not necessary that you should use this .Even you an apply lock on any other object which not an instance of this.

 synchronized(obj1){
 -------
 }

 synchronized(obj2){
 -------
 }

In a single method you can write something like above where you first acquire the lock on some object obj1 and then did work and released and then acquire the lock on obj2.

amicngh
  • 7,831
  • 3
  • 35
  • 54
0

Aaah, the infamous Avoid synchronized(this) in Java?.

There is, actually, no difference between

public synchronized void doThis() {


}

and

public void doThis() {
    synchronized (this) {

    }
}

Except on the bytecode level. Both address thread-safety. They both pose a problem as you can potentially introduce deadlock, if (for example, you set a lock of the same class, within its synchronized block).

If you're worried about deadlock, then a dedicated lock should be used, like so:

public class MyClass {
    private final Object lock = new Object();   //Must be final

    public void doThis() {
        synchronized (lock) {

        }
    }
}

Alternatively, use the Java java.util.concurrent.Lock interface and the java.util.concurrent.locks.ReentrantLock implementation to do basically lock..

Community
  • 1
  • 1
Buhake Sindi
  • 87,898
  • 29
  • 167
  • 228