Search code examples
javamultithreadingrunnableexecutorservice

Problem with thread synchronizing in Java


I'm well aware that this might be considered a duplicate, however I ran through many answers considering my problem here I can't come up with a solution.

I synchronized my runnable with an object shared by multiple threads and explicitly synchronized the method I am using inside, but the outcome of the program is always 3000.

I tried locking the Counter class but it won't change a thing. Could anyone explain me why none of my actions work in this particular example?

    public static void zad3() {
        var counter = new Counter();

        var toRun = new Runnable() {
            @Override
            public void run() {
                synchronized (counter) {
                    for (var i = 0; i < 1000; i++) {
                        counter.add(1);
                    }
                }
            }
        };

        var t1 = new Thread(toRun);
        var t2 = new Thread(toRun);
        var t3 = new Thread(toRun);

        t1.start();
        t2.start();
        t3.start();

        try {
            t1.join();
            t2.join();
            t3.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

        System.out.println("counter = " + counter.getCount());
   }
public class Counter {
    protected long count_ = 0;

    public synchronized void add(long value) {
        count_ += value;
    }

    public long getCount() {
        return count_;
    }
}

edit: As suggested the problem was in the loop being constantly ran a 1000 times by each of the threads. My solution:

        var toRun = new Runnable() {
            @Override
            public void run() {
                synchronized (counter) {
                    for (var i = counter.getCount(); i < 1000; i++) {
                        counter.add(1);
                    }
                }
            }
        };

Solution

  • Well you have synchronized the complete for loop around the "counter" variable which means that each thread will run tthe block once. 3 X 1000 = 3000

    this block will be executed once per thread

     for (var i = 0; i < 1000; i++) {
                            counter.add(1);
     }
    

    UPDATE: judging from your comments that you want interrupt on 1000 example code can be:

     t1.start();
     t2.start();
     t3.start();
    
    while(counter.getValue()<1000) {
        Thread.sleep(20)
    }
    

    Annother suggestion:

    public class Incremetor extends Runnable {
       Counter counter;
    
    public Incremetor(Counter counter) {
        this.counter = counter;
    }
    public void run() {
       counter.increment();
    }
    
    }
    
    ExecutorService executorService = Executors.newFixedThreadPool(8); // this mean 8 threads in total to do your runnables.
    for (int i=0;i<1000;++i) {
         executorService.submit(new Incrementor(counter));        
    }