1

There is a lot of material on stack-overflow about synchronization, but I still haven't acquired quality content about deciding which object to use as an intrinsic lock. Can some one actually make a good answer as a rule of thumb?

So should I choose 'monitor' as an instance variable or local variable or instance owning the method? All three of them do the job well. Also primitive value wrapper classes use 'pools' so no problem there as well, as threads 'attack' the same lock.

So why is it better to do this (this):

class A {
    void methodA(){
        synchronized (this){
            //some code    
        }
    }
}

over this(instance variable):

class A {
    String monitor = "monitor";
    void methodA(){
        synchronized (monitor){
            //some code
        }
    }
}

or over this(local variable):

class A {
    void methodA(){
        String monitor = "monitor";
        synchronized (monitor){
            //some code
        }
    }
}

They all work fine/same. So why did I read that I should avoid local variables when they implicitly use pools to store objects? What matter does the scope of variables make in this case?

Thanks!

Ana Maria
  • 475
  • 2
  • 11
  • They do *not* 'all work fine/same'. Synchronizing on a local variable accomplishes exactly nothing, as each caller has his own value. Synchronizing on an instance member rather than the instance allows you finer-grained control: you can have different monitors for different operations that don't interfere with each other. – user207421 Aug 06 '20 at 23:53

1 Answers1

2

You should avoid using the monitor of objects stored in local variables because typically only the current thread has access to objects stored in local variables. But since in this particular case, the local variable actually holds a globally shared object from the constant pool, you don't suffer from that particular problem.

The problem with using monitors of constant pool objects like here:

String monitor = "monitor";
void methodA() {
    synchronized (monitor){
        //some code
    }
}

... is that there is only one pooled constant object.

Two different threads operating on two different instances of class A cannot enter the synchronized block in methodA at the same time, even if you've ensured it should be safe (for example, you don't touch static shared state).

What's even worse: there might be some other class B somewhere else, which also happens to synchronize on the constant "monitor" string. Now a thread using class B will block other, unrelated, threads from using class A.

On top of that, it's incredibly easy to create a deadlock because you are unknowingly sharing locks between threads.

Joni
  • 108,737
  • 14
  • 143
  • 193
  • Can I ask you why did you say it should be safe in your 1st paragraph? In the given case we would synchronize all instances using that method and not just one instance with (this). What if we had static variable that all threads share? Then it would ruin program, wouldn't it? That is why we need to have 'common' instance variable as a 'monitor'. Am I correct? – Stefan Jankovic Aug 06 '20 at 23:59
  • Yes in that scenario you would need a lock that's common to all instances of the class. I've qualified the statement to clarify. – Joni Aug 07 '20 at 00:10