Why this synchronization doesn't work

[ShihChao Lin]

Hi:
Can anyone tell me why the synchronized for following code does not work?
public class SyncTest extends Thread{
StringBuffer letter = new StringBuffer("A");
private synchronized void printChar() {
char c;
c = letter.charAt(0);
for (int i=0; i< 5; i++) {
System.out.print(c);
}
System.out.println();
c++;
letter.setCharAt(0, c);
}
public void run(){
printChar();
}
public static void main(String[] args) {
SyncTest t1 = new SyncTest();
SyncTest t2 = new SyncTest();
t1.start();
t2.start();
}
}
I was hoping the output will be
AAAAA
BBBBB
But the result was
AAAAAAAAAA
Thanks for your help

[Michael Morris]

Hi Lin,
Welcome to JavaRanch. The problem here is that you have two different instances of the StringBuffer, both being initialized with "A". If you declare the StringBuffer static, then it will do what you expect.
Michael Morris

[John Smith]

The two threads that you created are absolutely independent and do not share any data. You might as well remove the synchronized qualifier and the result will be identical. To see the results of synchronization, you should create one object, pass it to two or more threads, and invoke the methods of that object from these threads.
Eugene.

[John Smith]

Michael, you beat me again? By a few seconds?
Eugene.

[Michael Morris]

Eugene's explaination is clearer than mine and more to the point of your question. Either way, the point of synchronization is to protect a single object from concurrent access and a race condition.
Michael Morris

[Michael Morris]

Yea, talk about threads in a race condition. But as I noted, yours was the better post.
Michael Morris

[ShihChao Lin]

Thanks. I think I got your point.

[Andrew Trumper]

Originally posted by ShihChao Lin:
Hi:
Can anyone tell me why the synchronized for following code does not work?
I was hoping the output will be
AAAAA
BBBBB
But the result was
AAAAAAAAAA
Thanks for your help

What you ment to do was:

public class SyncTest extends Thread{
static StringBuffer letter = new StringBuffer("A");
private static synchronized void printChar() {
char c;
c = letter.charAt(0);
for (int i=0; i< 5; i++) {
System.out.print(c);
}
System.out.println("");
c++;
letter.setCharAt(0, c);
}
public void run(){
printChar();
}
public static void main(String[] args) {
SyncTest t1 = new SyncTest();
SyncTest t2 = new SyncTest();
t1.start();
t2.start();
}
}

[huang zhiguo]

Is the following codes better?

public class SyncTest implements Runnable{
StringBuffer letter = new StringBuffer("A");
private synchronized void printChar() {
char c;
c = letter.charAt(0);
for (int i=0; i< 5; i++) {
System.out.print(c);
}
System.out.println();
c++;
letter.setCharAt(0, c);
}
public void run(){
printChar();
}
public static void main(String[] args) {
SyncTest thread=new SyncTest();
Thread t1 = new Thread(thread);
Thread t2 = new Thread(thread);
t1.start();
t2.start();
}
}

[chi Lin]

I agree with Huang's point, then I try some experiments with the code, I found something quite confusing here.
In the following code, SyncTest still extends Thread, however, in main() I use Huang's style to generate two threads.
the code compiles & generates expected result.
AAAAA
BBBBB
What I don't get, why those two lines compiles ?
Thread does not have such constructor that take a thread object or its subclass as argument.
Thread t1 = new Thread(thread);
Thread t2 = new Thread(thread);

public class SyncTest extends Thread{
   StringBuffer letter = new StringBuffer("A");
   private synchronized void printChar() {
      char c;
      c = letter.charAt(0);
      for (int i=0; i< 5; i++) {
         System.out.print(c);
      }
      System.out.println();
      c++;
      letter.setCharAt(0, c);
   }
   public void run(){
     printChar();
   }
   public static void main(String[] args) {
     SyncTest thread=new SyncTest();
     Thread t1=new Thread(thread); // compiles ? 
     Thread t2 = new Thread(thread);// compiles ?
     t1.start();
     t2.start();
   }
}

[John Smith]

What I don't get, why those two lines compiles ? Thread does not have such constructor that take a thread object or its subclass as argument.
The Thread class has the following constructor: Thread(Runnable target).
Since every instance of Thread implements Runnable, it can be used as an argument for that constructor.
Eugene.
[ April 25, 2003: Message edited by: Eugene Kononov ]

[chi Lin]

Thanks for the clarification.

[Ilja Preuss]

What would also have worked is using a shared object as synchronization monitor, for example the class instance:

private void printChar() {
char c;
synchronized (SyncTest.class) {
c = letter.charAt(0);
for (int i=0; i< 5; i++) {
System.out.print(c);
}
System.out.println();
c++;
letter.setCharAt(0, c);
}
}

[suvasis mukherjee]

hi
huang zhiguo solution is better because of the following reasons:
1. When you subclass Thread as in your case, the Thread object is just ab object that happens to point to the actual thread(a kernel, structure etc). Once a thread exits("stopped"), it's gone. It can not be restarted. Moreover, your intention is not to change nature of thread, so don't subclass it.
2. Whereas a Runnable may be used with as many threads as you like. this is a better solution.
Also, instead of sych the whole thread, you can sych only the loop portion i.e

Object o = new Object();
synchronized(o) {
for (int i=0; i< 5; i++) {
System.out.print(c);
}
}
suvasis

Originally posted by ShihChao Lin:
Hi:
Can anyone tell me why the synchronized for following code does not work?
public class SyncTest extends Thread{
StringBuffer letter = new StringBuffer("A");
private synchronized void printChar() {
char c;
c = letter.charAt(0);
for (int i=0; i< 5; i++) {
System.out.print(c);
}
System.out.println();
c++;
letter.setCharAt(0, c);
}
public void run(){
printChar();
}
public static void main(String[] args) {
SyncTest t1 = new SyncTest();
SyncTest t2 = new SyncTest();
t1.start();
t2.start();
}
}
I was hoping the output will be
AAAAA
BBBBB
But the result was
AAAAAAAAAA
Thanks for your help