1

i am unable to achieve synchronization of thread here .i used a synchronized method "meth" her . so according to definition only one thread should enter at a single time and print my desired output . but this is not happening . Need help. Thank you.

class ABC {

    synchronized public void meth(String msg) {
        System.out.print("[" + msg);
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            System.out.println(Thread.currentThread().getName() + " Thread Interrupted");
        }
        System.out.println("]");
    }
}

class SyncMethod implements Runnable {

    ABC a = new ABC();
    Thread t;

    SyncMethod(String s) {
        t = new Thread(this, s);
        t.start();
    }

    public void run() {
        a.meth(t.getName());
    }

    public static void main(String args[]) {
        new SyncMethod("Hello");
        new SyncMethod("Synchronized");
        new SyncMethod("World");
    }
}

Current Output :

        [Hello [Synchronized [World] ] ] ]

Desired Output :

        [Hello]
        [Synchronized]
        [World]
Andy Turner
  • 137,514
  • 11
  • 162
  • 243
Night.star
  • 13
  • 3
  • The order of Thread execution is unknown. Since you initiate 3 threads in the wild, they could start in any arbitrary order – ShayHaned Jul 05 '17 at 07:23
  • It might be helpful for you to edit your question to include the output you're actually seeing, and what you expect to see. – DaveyDaveDave Jul 05 '17 at 07:36
  • 1
    @DaveyDaveDave it was there all along, just hidden in the code block. Edited. – Andy Turner Jul 05 '17 at 07:37
  • Note that you won't necessarily see that output, even with correct synchronization. You might see `[World] [Hello] [Synchronized]`. – Andy Turner Jul 05 '17 at 07:38
  • Also: don't start threads in the constructor. That makes it impossible to use the `Runnable` in other circumstances, e.g. submitting to an `ExecutorService`. Call `new Thread(new SyncMethod(...)).start();` instead. – Andy Turner Jul 05 '17 at 07:40

3 Answers3

2

When you synchronize on an instance method:

class ABC
{
    synchronized public void meth(String msg)
    { ... }
}

this is the same as:

class ABC
{
    public void meth(String msg) {
        synchronized (this) { ... }
    }
}

i.e. the monitor you're acquiring is the instance of that class. But you're creating 3 separate instances (each thread calls new ABC()), and acquiring the monitor on each of them separately, so the synchronization is effectively a no-op.

You need to synchronize on a common object. For example:

class ABC
{
    public void meth(String msg) {
        synchronized (ABC.class) { ... }
    }
}

In general, synchronizing on a class is ill-advised, though, because any code anywhere in your program can acquire that monitor, so you might get unexpected contention.

Instead, pass in a lock object:

class ABC
{
    private final Object lock;

    ABC(Object lock) { this.lock = lock; }

    public void meth(String msg) {
        synchronized (lock) { ... }
    }
}

and pass the same lock to all ABC instances that you want to be synchronized.


But rather than doing this, as urag points out in his/her answer, you can pass the same instance of ABC to all of your threads, rather than creating a new instance in each SyncMethod. Sometimes the simplest solutions just escape me when I'm writing an answer ;)

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
  • *pass the same lock to all ABC instances* what is the benefit of doing that rather than using a static object variable? – Scary Wombat Jul 05 '17 at 07:27
  • @ScaryWombat let's say you've got four instances of `ABC`, working in pairs, as in you care about `a` and `b` being synchronized; and `c` and `d` being synchronized; but you don't care if `a` and `c` are. If you are using a static variable, you get synchronization across all 4, which may lead to unnecessary contention. – Andy Turner Jul 05 '17 at 07:29
  • 2
    Thanks for the explanation - will adopt from now on – Scary Wombat Jul 05 '17 at 07:31
  • @ScaryWombat (it's an example of the general problem with mutable global state). – Andy Turner Jul 05 '17 at 07:31
1

Look here you are creating new instance of ABC each time you calling the constructor of SyncMethod so you have 3 copies of the class each called by different thread so they don't compete for the monitor so what you need is to use the same ABC object for all 3 calls here is the solution

class SyncMethod implements Runnable {
Thread t;
ABC a;

SyncMethod(String s, ABC a) {
    this.a = a;
    t = new Thread(this, s);
    t.start();

}

public void run() {
    a.meth(t.getName());
}

public static void main(String args[]) {
    ABC a = new ABC();
    new SyncMethod("Hello", a);
    new SyncMethod("Synchronized", a);
    new SyncMethod("World", a);
}

}

urag
  • 1,228
  • 9
  • 28
  • that was very helpful. Thank you :) – Night.star Jul 05 '17 at 11:55
  • then why not just make the object of ABC static , this will make the code shorter ;) . Check my answer. – Night.star Jul 05 '17 at 12:13
  • Yes it would give the same result so for this particular example it's fine but don't use static lightly it is not always the best approach here is a good link that talks about statics https://stackoverflow.com/questions/2671496/java-when-to-use-static-methods – urag Jul 05 '17 at 13:22
0

i have come with a better solution myself. As @urag says that we need a single object to call the synchronized method while i was creating a new object evrytime thus calling 3 synchronized methods . we can make the object of class ABC static and thus it will not respawn itself and the code will also become shorter.I have checked this works.

    class ABC
{
    synchronized public void meth(String msg)
    {
        System.out.print("[" + msg);
        try
        {
            Thread.sleep(1000);
        }
        catch(InterruptedException e)
        {
            System.out.println(Thread.currentThread().getName() + " Thread Interrupted");
        }
        System.out.println("]");
    }
}
class SyncMethod implements Runnable
{
    static ABC a = new ABC();
    Thread t;
    SyncMethod(String s)
    {
        t= new Thread(this,s);
        t.start();
    }
    public void run()
    {
        a.meth(t.getName());
    }
    public static void main(String args[])
    {
        //ABC a = new ABC();
        new SyncMethod("Hello");
        new SyncMethod("World");
        new SyncMethod("Synchronized");
    }
}
Night.star
  • 13
  • 3