CODEWITHSUNDEEP
javamultithreading
ken
ken

Reputation: 471

Synchronize across threads

The following code should assure that access to "sync" is synchronized across all threads.

According to the a output this is not always the case, notice how Thread-3 and Thread-4 are reading the same value of sync.

Am I missing something in the code?

[Thread-0] before value of sync is 0
[Thread-0] after value of sync is 1
[Thread-3] before value of sync is 1
[Thread-3] after value of sync is 2
[Thread-4] before value of sync is 1
[Thread-4] after value of sync is 3
[Thread-2] before value of sync is 3
[Thread-2] after value of sync is 4
[Thread-1] before value of sync is 4
[Thread-1] after value of sync is 5

here the code:

package com.mypackage.sync;

public class LocalSync implements Runnable {

    private Integer sync = 0;

    public void someMethod() {
        synchronized (sync) {
            System.out.println("[" + Thread.currentThread().getName() + "]" + " before value of sync is " + sync);
            sync++;
            System.out.println("[" + Thread.currentThread().getName() + "]" + " after value of sync is " + sync);
        }
    }

    @Override
    public void run() {
        someMethod();
    }

    public static void main(String[] args) {

        LocalSync localSync = new LocalSync();
        Thread[] threads = new Thread[5];
        for (int i = 0; i < threads.length; i++) {
            threads[i] = new Thread(localSync, "Thread-" + i);
            threads[i].start();
        }

    }
}

Upvotes: 1

Views: 177

Answers (3)

SMA
SMA

Reputation: 37023

Integer is immutable class and when you are doing sync++, you are assigning a new reference to Sync and your other thread might hold the reference to an older sync and hence the issue with multithreading. Try defining a simple MUTEX of say INTEGER like :

private final Integer MUTEX = new Integer(1);

And use it instead of synchronizing on sync.

Upvotes: 3

Ludovic Feltz
Ludovic Feltz

Reputation: 11916

You should synchronize on an Object 

private Object synchObj = new Object();
private Integer sync = 0;

public void someMethod() {
    synchronized (synchObj) {
        System.out.println("[" + Thread.currentThread().getName() + "]" + " before value of sync is " + sync);
        sync++;
        System.out.println("[" + Thread.currentThread().getName() + "]" + " after value of sync is " + sync);
    }
}

...

Upvotes: 0

JB Nizet
JB Nizet

Reputation: 691635

You're constantly changing the sync object on which all threads are supposed to be synchronized. So, effectively, they aren't synchronized at all. Make your sync variable final, as every lock should be, and you'll see that your code doesn't compile anymore.

Solution: synchronize on another, final object, or use an AtomicInteger and change its value, or synchronize on this (i.e. make the method synchronized).

Upvotes: 6

Related Questions