Java synchronize data between threads in Executor Service pool

Question

I am trying to synchronize a int counter between few threads (6). Below is my worker class. I am trying to synchronize my private field countertest:

public class DbWorker implements Callable {

    private final Object lock = new Object();
    private int countertest = 0 ;

    private  boolean  IsActive(Integer act)
    {
        synchronized(lock){ 
        boolean exists = false;
        countertest++;
            ....
       System.out.println("countertest  IS :  " + countertest );
         }
       return true;
    }

     public void run() {
        ....
        IsActive(act):
        ...
     }

}

In my main class I am creating threads using for loop and ExecutorService. Please see below:

private ExecutorCompletionService<Integer> _objQueue ;

 for(int j = 0; j < 6; j++){
     _objQueue.submit( new DbWorker( "SOME PARAMETER" , _snapshots.get(j) , j )    );
}

My countertest variable is not synced it is printing different number (non-sequential). What am I doing wrong?

The "duplicate", IMO, is a bad question with a bad answer. Bad because it promotes the idea that objects can be `static` or not static. (They can't: Only _variables_ can be `static` or not static.) And, bad because it promotes the idea that `static` makes a difference in locking. (It doesn't. What matters is, two threads can not lock the same object at the same time. Whether the `synchronized` statement obtains the reference from a static variable, from a non-static variable, or by calling a method is irrelevant.) — Solomon Slow, Oct 16 '15 at 13:08

Sotirios Delimanolis · Accepted Answer · 2015-10-16T01:04:26.307

3

Your lock

private final Object lock = new Object();

is an instance field. In other words, there's one per DbWorker object.

You then submit DbWorker instances to for execution

_objQueue.submit( new DbWorker( "SOME PARAMETER" , _snapshots.get(j) , j )    );

Each thread has its own DbWorker and therefore its own lock. Threads are never contending with any other thread.

Create one DbWorker and share it among threads, ie. submit the same DbWorker instance every time.

edited Oct 16 '15 at 01:04

answered Oct 16 '15 at 00:54

Sotirios Delimanolis

274,122
60
696
724

I am passing an argument for each DbWorker . I cannot use the same DbWorker. – Kenner Dev Oct 16 '15 at 00:58
@KennerDev Then share the counter and the lock between them. Use an `AtomicInteger` if anything. – Sotirios Delimanolis Oct 16 '15 at 00:58
My lock is final already .What else do I need to do ? – Kenner Dev Oct 16 '15 at 01:01
@KennerDev Share it between threads. – Sotirios Delimanolis Oct 16 '15 at 01:01
Do u mean using volatile ? – Kenner Dev Oct 16 '15 at 01:03
Oh mean make the lock Static – Kenner Dev Oct 16 '15 at 01:05
@KennerDev No. Re-read my first paragraph. The lock object (and the counter) are instance variables. You create 6 (`j`) instances. There's nothing shared among them. – Sotirios Delimanolis Oct 16 '15 at 01:05
@KennerDev It wouldn't be enough for the lock to be `static`. The counter would have to be as well. – Sotirios Delimanolis Oct 16 '15 at 01:05
@KennerDev Assuming you wanted sequential numbers. – Sotirios Delimanolis Oct 16 '15 at 01:08
Yes . I am testing it and will let you know in a bit ( slow pc) – Kenner Dev Oct 16 '15 at 01:09
Sotirios Delimanolis. Its is working.Thanks much . I missed a very obvious point.Marking it as answer – Kenner Dev Oct 16 '15 at 01:13

score 2 · Answer 2 · edited Oct 16 '15 at 01:02

2

Because you creating an instance in the loop where where countertest and lock are instance variable and hence every object will get a new one.

countertest will always be 1 in this case, it is not shared between instances
multiple threads aren't acquiring the same lock, so it is useless.

edited Oct 16 '15 at 01:02

Nayuki

17,911
6
53
80

answered Oct 16 '15 at 00:55

Sleiman Jneidi

22,907
14
56
77

I am passing an argument for each DbWorker . I cannot use the same DbWorker. – Kenner Dev Oct 16 '15 at 00:58
I am just answering your question, "why it is not working". I don't know what you trying to do but your code looks so broken anyways – Sleiman Jneidi Oct 16 '15 at 01:00

Java synchronize data between threads in Executor Service pool

2 Answers2