Does synchronized (this) lock only the synchronized block or all the "this" code?

Question

public class ObjectCounter {
    private static long numOfInstances = 0;
    public ObjectCounter(){
        synchronized(this){
        numOfInstances++;
        }
    }
    **public static synchronized long getCount(){
        return numOfInstances;
    }**
//vs//
    **public static long getCount(){
        return numOfInstances;
    }**
}

if I'll run few threads, some of them call the static function getCount() and some of them create new instances. I want to get in each call to getCount() the real number of instances at the time.

1. Is there a difference between the two options in the code?
2. If I lock "this" shouldn't it mean that I can't call getCount() until the constructor exits the synchronized block (lets say if I don't write synchronize on the getCount()).
3. if I do a synchronized block in some place in the code, does it lock only the synchronized block or all the "this" code?
4. From here down EDIT: thank you all, it was very helpful, but I have a few more questions following your answers.
5. If I understand correctly, the synchronized(this) block doesn't effect (or connected to) the static synchronized function (in lock terms not the numOfInstances increment)?
6. is there a better option to make the increment and the getCount() function Thread-safe? (like open a static object and do synchronized(obj) instead synchronized(this) - friend suggested).
7. If I had a f1() method (non-static) in ObjectCounter class, while one thread is in the synchronized(this) can other thread enter f1() block (not a synchronized class or have synchronized block inside)?
8. If I had a f1() method (non-static) and f2() method (non-static) in ObjectCounter, in f1() I have synchronized(this) block. while one thread is in the synchronized(this) block, can other thread enter f1() block (not a synchronized class or have synchronized block inside)? (lets say both of the threads "working" on the same instance)

`

Chordate

You must never allow calls to an instance method (e.g., getCount()) of any object before the constructor for that instance has returned. The only way that's even possible is if the constructor publishes this to another thread. (also known as "leaking this from a constructor"). If you think you have to write synchronized(this) inside a constructor, then you probably are making a huge mistake. – Meda

Please don't edit your question in order to add more questions (or, even worse, replace your existing questions) after you have gotten a correct and complete answer to your question. Open a new question for that or comment on the correct answer with your follow-up questions. – Ethnic

All the answers are very good and helpful, however I can pick only one that helped me, and I don't think it's fair to pick only one. If it's a must I'll pick one. – Chordate

Answer 1

Using synchronized means in order for a thread to execute that block or method, it has to acquire a lock referenced (explicitly or implicitly) by that block or method. For the static synchronized methods, that lock is the monitor on the class object. For the synchronized(this) block, the lock used is the monitor on the current instance. Sharing of locks between multiple methods or blocks is what enforces atomicity and memory visibility of updates, also the shared lock provides a shared communication path through which waiting and notification can take place.

Since the static synchronized blocks use a different lock from that used by the block in the constructor, entering a static synchronized block is not blocked by another thread's accessing the block that requires acquiring the lock on the current instance, and the synchronized block in the constructor has no effect on anything, the lock acquisition will always be uncontended. More importantly here, changes made by one thread in the constructor may not get seen by other threads using the getter. Synchronization affects both locking and memory visibility.

This changed version would work:

public class ObjectCounter {
    private static long numOfInstances = 0;
    public ObjectCounter(){
        synchronized(ObjectCounter.class){
            numOfInstances++;
        }
    }
    public static synchronized long getCount(){
        return numOfInstances;
    }
}

because the getter and the incrementing block are using the same lock. Making the different threads acquire the same monitor ensures that the change to the counter gets safely published so that another thread accessing the getter can see the updated value.

The synchronized keyword says, "you have to acquire a lock before you can enter", where for the method the lock is assumed: with the static keyword on the method it's the monitor on the class, without a static keyword it's the monitor on the current instance. For locking to work correctly the different blocks and methods need to use the same lock. There is arguably too much syntax sugar and too much making things convenient in how Java was designed: allowing implicit choice of locks and putting the monitor on java.lang.Object can cause confusion.

WRT your question #6: For what you're doing here you'd be better off with an AtomicLong. Use synchronized blocks for coordinating multiple changes that need to take place without interference from other threads.

Questions #3, #7 and #8 seem very similar: If a method/block isn't attempting to acquire a lock, nothing prevents threads from executing that method/block. The object as a whole doesn't get any protection, using the synchronized methods or blocks to enforce locking is what does the protecting. Think less in terms of "using the synchronized keyword" and more in terms of what lock threads need to acquire.

Molybdate

Bad Dog! Noobs need to understand that locks are not associated with blocks, not associated with methods, and not associated with variables. I've lost count of how many times I've seen them ask, "how is it possible that two threads got into this same synchronized block/method at the same time?" Or, "How is it possible that two threads both synchronized on count at the same time?" They need it spelled out very clearly that the only thing synchronized prevents is two threads locking the same instance at the same time. – Meda

@james: I reworded to try to make this clearer. Suggestions for improvements or just edits are welcome. btw my gravatar is of a cat, albeit a crudely-rendered one. – Molybdate

Oh Dear! I always thought it was a Schnauzer of some kind. Sorry for the intemperate remark, but it was the part where you spoke of "the lock associated with a block" that got me going. I probably say the same thing to my peers sometimes, but my peers all know that the association between the lock and the code is an aspect of how my program is designed, not because of how the language works. – Meda

@james: np, i think the wording is improved as a result, thanks. – Molybdate

@NathanHughes what about synchronized on different lock is it guaranteed visibility ? (I know it's not thread safe but are changes visible for threads in different lock) – Chare

Answer 2

1. Yes there is a difference in the options. In the above option, two threads cannot call getCount() at the same time, in the below one they can.

2. Yes, that is correct. There can only be one thread at the same time holding a lock on an object.

3. Each object has its own lock. So it lock all synchronized (this) block of that object.

Note, however, that each object has a lock of its own and also each class has a lock of its own. In the constructor you use the object lock to access a static (class) variable, while in getCount() you use the class lock. That means that your code is not thread-safe!

Carmagnole

Answer 3

synchronized steps:

1. Check if object lock is already acquired. If so, proceed into synchronized block/method
2. Attempt to aquire lock. If the lock has already been acquired by another thread, then the thread will wait for the lock to be released, at which point it will go through the cycle (2.) again

Peanut

Answer 4

Is there a difference between the two options in the code?

Yes, there is clear difference. In first, you are synchronizing threads access to getCount() method on the class object of ObjectCounter. While in second you are not.

If I lock "this" shouldn't it means that I can't call getCount() until the contractor exit the synchronized block (lets say if I don't write synchronize on the getCount()).

Since there is only one lock of an object (class locks are different which are held through using static keyword along with synchronized), so if some other thread is acquiring that lock either because of synchronized(this){ or because of this synchronized long getCount(){ then the new thread trying to acquire the lock has to wait until the previous thread has released the lock.

Now since in your case, you are doing, static synchronized long getCount(){, so, its locking becomes different from synchronized(this){. Which means that if some thread is acquiring a lock because of synchronized(this){ and some other thread is trying to invoke getCount() then that thread will not be blocked.

if I do a synchronized block in some place in the code, does it locks the only the synchronized block or all the "this" code?

1. Non-static synchronization: If you do synchronized block in some place in the code and it is non-static public synchronized long getCount(){, then also the lock of your object will be held, so new thread trying to acquire the lock has to wait until the previous thread has released the lock.

2. Static synchronization: If you do synchronized block in some place in the code and it is static public static synchronized long getCount(){, then it will have no effect on lock of non-static sychronization.

Bottom line:

• There is only and only one lock for one object, and if that lock is acquired by some thread then other threads has to wait until that lock released.

• Then there is a class lock, which is held if static keyword is used along with synchronized keyword.

Paraphernalia

Down vote without any explanation is very poor display of spirit. – Paraphernalia

@jameslarge Oh ok. My deepest apologies. – Paraphernalia

@NathanHughes "Answers need to get to the point fast." Yeah, golden words!!! 2 unqualified black spots :( – Paraphernalia

@hagrawal I took a look at the edit history of your answer. The first version of your answer said so if some other thread is acquiring that lock either because of synchronized(this){ or because of this static synchronized long getCount(){ then the new thread trying to acquire the lock has to wait until the previous thread has released the lock. That's why you may have got downvoted because that's a clearly incorrect statement. I am with the downvoters on this one. – Kalgoorlie

@CKing I used OP's code for explanation and missed to remove the static from it, which I realized later and corrected. I guess I paid the price of misses. Thank for your note though. I agree with you on your position with down-voters, but I guess I covered everything which OP was looking for, though after few edits. – Paraphernalia

@hagrawal How about this statement in your first version : In first, you are synchronizing threads access to getCount() on the object of ObjectCounter. While in second you are not. Was this a typo too? I highly doubt it since there is no scope for a typo in this statement. Bottom line. You got downvotes because of a dangerously incorrect answer. This is documented as one of the cases where downvotes make sense. The black spots are qualified. By the time you corrected your answer, it might have been too late as downvotes can only be retracted for a limited amount of time. – Kalgoorlie

@CKing Yes, it was not a typo. It was an elaboration, so that OP doesn't get confused. – Paraphernalia

@CKing "This is documented as one of the cases where downvotes make sense." Where it is documented, can I read it? – Paraphernalia

@hagrawal The elaboration was incorrect. Neither of the two forms of the getCount method will acquire a lock on the object of ObjectCounter. It's actually confusing the OP rather than avoiding confusion. Here's the documentation for downvoting. In all fairness, the users could not retract their downvote maybe because the time limit on retracting a downvote expired. – Kalgoorlie

Let us continue this discussion in chat. – Paraphernalia

Oh Ok. I thought this particular case was used for some kind of documentation. – Paraphernalia