What is wrong with the synchronization in following code

Question

SYNCHRONIZATION

I have declared a class b which has a synchronized method which is accessed in class c:

class b {
    String msg;

    public synchronized void foo() {
        System.out.print("[" + msg);
        try {
            Thread.sleep(1000); // Threads go to sleeep

        } catch (InterruptedException e) {
            System.out.println("Caught" + e);
        }
        System.out.println("]");
    }
}

class a implements Runnable {
    b ob;

    Thread t;

    a(String msg, b obb) {
        ob = obb;
        ob.msg = msg;
        t = new Thread(this); // creating a thread
        t.start();
    }

    public void run() {
        ob.foo(); // calling method of class b
    }

    public static void main(String... a) {
        b obb = new b();
        a ob = new a("Hello", obb); /* PASSING */
        a ob1 = new a("Synch", obb); /* THE */
        a ob2 = new a("World", obb);/* MESSAGE */

        try {
            ob.t.join();
            ob1.t.join();
            ob2.t.join();
        } catch (InterruptedException e) {
            System.out.println("Caught" + e);
        }
    }
}

I am expecting the output:

[Hello]
[Synch]
[World]

But the code gives:

[World]
[World]
[World]

Help me with some suggestions. I am a naive JAVA user.

There is too much missing - Why should it write anything, where is foo being called, and why should it say Hello Synch World? — Humungus, Oct 11 '13 at 08:59
Java is pass by reference for objects, so `ob.msg=msg;`is overwriting the common `b` message (see http://stackoverflow.com/questions/40480/is-java-pass-by-reference) — , Oct 11 '13 at 09:00

score 1 · Answer 1 · answered Oct 11 '13 at 09:23

1

use the following code to get the expected answer.

class b { // String msg;

public void foo(String msg) {
    System.out.print("[" + msg);
    try {
        Thread.sleep(1000); // Threads go to sleeep

    } catch (InterruptedException e) {
        System.out.println("Caught" + e);
    }
    System.out.println("]");
}

}

public class Threading implements Runnable {

    b ob;
String msg;
Thread t;

Threading(String message, b obb) {
    ob = obb;
    msg = message;
    t = new Thread(this); // creating a thread
    t.start();
}

public void run() {
    synchronized (ob) {
        ob.foo(msg); // calling method of class b
    }

}

public static void main(String... a) {
    b obb = new b();
    Threading ob = new Threading("Hello", obb); /* PASSING */
    Threading ob2 = new Threading("World", obb); /* THE */
    Threading ob1 = new Threading("Synch", obb);/* MESSAGE */

    try {
        ob.t.join();
        ob1.t.join();
        ob2.t.join();
    } catch (InterruptedException e) {
        System.out.println("Caught" + e);
    }
}

}

answered Oct 11 '13 at 09:23

pnathan

713
3
9

thnx pnathan.... But in my code before msg gets overriden doesn't the value "Hello" gets printed in foo? And after the first thread sleeps, the control comes back to main() – Steve Oct 11 '13 at 09:24
no steve in your code what happened value is got overriden each time before it get printed because there is a sleep before printing the value so it prints the last value (i.e world) each time which is supplied. – pnathan Oct 11 '13 at 09:33
pnathan actually the first print statement is before sleep statement. Sorry I am really not getting this point – Steve Oct 11 '13 at 09:36
i think we can not predict that value get overriden or method get executed. – pnathan Oct 11 '13 at 09:46

Subir Kumar Sao · Accepted Answer · 2013-10-11T09:10:09.263

0

In the code you have set ob.msg=msg; msg gets overridden by the threads. So you have the same message for all the Threads.

In the constructor of each Thread of class a you are passing the same object of class b. Now all three threads are setting the value of msg of the instance of class b. So one value overrides the other. What you have is the value set by last thread i.e. World.

IMO: Save the msg as an instance variable in each thread and pass it to the foo method as a parameter.

And please follow Java naming convention i.e. Camel Casing

edited Oct 11 '13 at 09:10

answered Oct 11 '13 at 09:01

Subir Kumar Sao

8,171
3
26
47

Added explanation to the answer. – Subir Kumar Sao Oct 11 '13 at 09:10
But before msg gets overriden doesn't the value "Hello" gets printed in foo? And after the first thread sleeps the control comes back to main() – Steve Oct 11 '13 at 09:17
Its unpredictable whether the value gets overridden first or the foo method gets executed 1st. Either way to make ur code predicatable you need to pass the value to the foo method. – Subir Kumar Sao Oct 11 '13 at 09:32

score 0 · Answer 3 · answered Oct 11 '13 at 09:10

0

First of all reformat your code. It is very hard to read. Second when you call ob.msg = msg; it overrites msg value in ob and as it is not synchronized so you cannot actually predict what the output will be.

answered Oct 11 '13 at 09:10

Dia

271
1
13

What is wrong with the synchronization in following code

3 Answers3