Reputation: 11

Why synchronization is not working

problem here is synchronization is not working. My other thread gets executed even before my first thread has finished its execution in synchronized block. When I'm passing "someClass.class", then it works fine. Maybe my understading is wrong or maybe my program has some fault.

Main:

public class Main {
    public static void main(String[] args) {
        SharedObject object = new SharedObject();
        ClassA t1 = new ClassA("Thread 1", object);
        ClassA t2 = new ClassA("Thread 2", object);
        t1.start();
        t2.start();
    }

}

SharedObject:

class SharedObject {
    volatile int value = 10;
}

ClassA:

class ClassA extends Thread{
    SharedObject object;
    int data;

    public ClassA(String name, SharedObject object) {
        super(name);
        this.object = object;
    }

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

    public void name() {
        synchronized (this) {
            if(getName().equals("Thread 1")){
                System.out.println("Before modifying "+getName()+" value = "+object.value);
                object.value = 100;
                System.out.println("After modifying "+getName()+" value = "+object.value);
                data = object.value;
            }else{
                System.out.println("Before modifying "+getName()+" value = "+object.value);
                object.value = 200;
                System.out.println("After modifying "+getName()+" value = "+object.value);
                data = object.value;
            }
        }

    }
}

Output of the program:

Before modifying Thread 2 value = 10
Before modifying Thread 1 value = 10
After modifying Thread 2 value = 200
After modifying Thread 1 value = 100

Please help me in understanding this.

Upvotes: 1

Answers (4)

Jean Logeart

Reputation: 53859

You need to synchronize on object and not on this.

this is different for t1 and t2 because they are two different instances of the same class, hence the concurrent execution.

Upvotes: 2

Mathieu Fortin

Reputation: 1058

While most answers are correct, let me add some more explanations on the different types of synchronization you can do.

Synchronized method

For example:

public synchronized void name() {
    
        if(getName().equals("Thread 1")){
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 100;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }else{
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 200;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }
}

This makes sure that only one thread will execute this specific method at one point in time for that particular instance. Any other thread will wait at the method starting point.

Synchronized instance

For example:

public void name() {

    synchronized (this) {
        if(getName().equals("Thread 1")){
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 100;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }else{
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 200;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }
    }

}

This makes sure that only one thread will execute this specific block at one point in time for that particular instance. Any other thread will wait at the beginning of the block.

When the synchronized block encompasses the whole method, this is equivalent to the synchronized method.

Synchronized class

public void name() {

    synchronized (ClassA.class) {
        if(getName().equals("Thread 1")){
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 100;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }else{
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 200;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }
    }

}

This makes sure that only one thread will execute this specific block at one point in time for any instances of the class. Any other thread will wait at the beginning of the block. This is a possible fix to your problem.

Synchronizing mutex

While all above methods use an implicit mutex, a good way of synchronizing threads is to use your own mutex. This improves overall extensibility if you go deeper into concurrency implementations.

For example:

private staic final Object mutex = new Object();

public void name() {

    synchronized (mutex) {
        if(getName().equals("Thread 1")){
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 100;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }else{
            System.out.println("Before modifying "+getName()+" value = "+object.value);
            object.value = 200;
            System.out.println("After modifying "+getName()+" value = "+object.value);
            data = object.value;
        }
    }

}

Upvotes: 3