Koray Tugay
Koray Tugay

Reputation: 23780

Why is this synchronisation not working?

This is my code:

public class TestClass {
    public static void main(String[] args) throws Exception {
        Thread threadOne = new Thread(new SomeRunnable("x"));
        Thread threadTwo = new Thread(new SomeRunnable("y"));
        threadOne.start();
        threadTwo.start();
    }
}

public class SomeRunnable implements Runnable {
    private String name;
    public SomeRunnable(String name) {
        this.name = name;
    }
    @Override
    public void run() {
        for(int i=0;i<50;i++) {
            NameShouter.shoutName(name);
        }
    }
}

public class NameShouter {
    public static void shoutName(String name) {
        synchronized (System.out) {
            System.out.print(name);
        }
    }
}

The output I will get is:

xxxxxxxxxyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

I am expecting that System.out will be synchronized but it is not. Why? And how can I make this code work that it will output:

xxxxxxxxxxx....yyyyyyyy....

Upvotes: 0

Views: 51

Answers (2)

Sarthak Mittal
Sarthak Mittal

Reputation: 6104

ok try something like this :

public class TestClass {
public static void main(String[] args) throws Exception {
    TestClass test = new TestClass();
    Thread threadOne = new Thread(new SomeRunnable("x",test ));
    Thread threadTwo = new Thread(new SomeRunnable("y",test ));
    threadOne.start();
    threadTwo.start();
}
}

public class SomeRunnable implements Runnable {
private String name;
private TestClass test;
public SomeRunnable(String name, TestClass test) {
    this.name = name;
    this.test = test;
}
@Override
public void run() {
   synchronized(test){
    for(int i=0;i<50;i++) {
        NameShouter.shoutName(name);
    }
  }
}
}

public class NameShouter {
public static void shoutName(String name) {      
        System.out.print(name);        
}
}

OR if you don't want to create an object of TestClass you can do something like this:

public class Try {
public static void main(String[] args) throws Exception {
Try test = new Try();
Thread threadOne = new Thread(new SomeRunnable("x",Try.class));
Thread threadTwo = new Thread(new SomeRunnable("y",Try.class));
threadOne.start();
threadTwo.start();
}
}

class SomeRunnable implements Runnable {
private String name;
private Class test;
public SomeRunnable(String name, Class test) {
this.name = name;
this.test = test;
}
@Override
public void run() {
synchronized(test){
for(int i=0;i<50;i++) {
NameShouter.shoutName(name);
}
}
}
}

Upvotes: 2

Joey
Joey

Reputation: 354376

You are synchronizing every single invocation of shoutName, which just means that each name is written completely before the next one. If you want to shout all the same names one after another you need to move the synchronized around the loop:

public class SomeRunnable implements Runnable {
    private String name;
    public SomeRunnable(String name) {
        this.name = name;
    }
    @Override
    public void run() {
        synchronized (System.out) {
           for(int i=0;i<50;i++) {
                NameShouter.shoutName(name);
            }
        }
    }
}

public class NameShouter {
    public static void shoutName(String name) {
        System.out.print(name);
    }
}

Since you alluded that you potentially have different NameShouters that shout from different things (and you still need the capability to shout those names 50 times), you could introduce a method overload that shouts a number of times and pass the PrintStream to use as an argument:

public static void shoutName(String name, int count, PrintStream out) {
    synchronized (out) {
        out.print(name);
    }
}

Then you can just call shoutName("x", 50, System.out) for your current behaviour.

Over time you'll notice that just static methods make things cumbersome here, though, and you could just use NameShouter objects instead which have appropriate properties for the resource they are shouting to and whatever else is necessary.

If your resource is not just a PrintStream, e.g. if you want to shout names to images as well, maybe, then either wrap whatever else you want to shout to in a PrintStream (probably not the best idea because misusing an interface for certain things makes it somewhat hard to implement), or create your own interface, e.g.

public interface IResource {
    void print();
}

and wrap System.out and whatever else you're using appropriately into an IResource.

Upvotes: 2

Related Questions