java synchronized issue

Question

I'm having issues with Synchronized not behaving the way i expect, i tried using volatile keyword also:

Shared Object:

public class ThreadValue {
private String caller;
private String value;
public ThreadValue( String caller, String value ) {
    this.value = value;
    this.caller = caller;
}

public synchronized String getValue() {
    return this.caller + "     "  + this.value;
}
public synchronized void setValue( String caller, String value ) {
    this.caller = caller;
    this.value = value;
}
}

Thread 1:

class CongoThread implements Runnable {
    private ThreadValue v;
    public CongoThread(ThreadValue v) {
    this.v = v;

    }
    public void run() {
    for (int i = 0; i  10; i++) {
    v.setValue( "congo", "cool" );
    v.getValue();
    }
    }
}

Thread 2:

class LibyaThread implements Runnable {
    private ThreadValue v;
    public LibyaThread(ThreadValue v) {
    this.v = v;

    }
    public void run() {
    for (int i = 0; i  10; i++) {
       v.setValue( "libya", "awesome" );
       System.out.println("In Libya Thread " + v.getValue() );

    }
    }
}

Calling Class:

class TwoThreadsTest {
    public static void main (String args[]) {

    ThreadValue v = new ThreadValue("", "");
        Thread congo = new Thread( new CongoThread( v ) );
        Thread libya = new Thread( new LibyaThread( v ) );

    libya.start();
        congo.start();

    }
}

Occasionally i get "In Libya Thread congo cool" which should never happen. I expect only: "In Libya Thread libya awesome" "In Congo Thread congo cool"

I dont expect them to be mixed.

Answer 1

The calls can interleave like the following:

Thread 1 : v.setValue()
Thread 2 : v.setValue()
Thread 1 : v.getValue() // thread 1 sees thread 2's value
Thread 2 : v.getValue() // thread 2 sees thread 2's value

Answer 2

It could be in this order so it's correct:

 v.setValue( "libya", "awesome" );
 //context switch
 v.setValue( "congo", "cool" );
 //context switch
 System.out.println("In Libya Thread " + v.getValue() );

Therefore you DO have a race conditions in some meaning. Synchronized is acquire a lock whenever you trying to call synchronized method, so you need another way to imply synchronized access to variable. For example you can remove synchronized from methods and do the following:

public void run() 
{
  for (int i = 0; i  10; i++) 
  {
   synchronized(v)
   {
      v.setValue( "caller", "value" );
      v.getValue();
   }
  }
}

Answer 3

The calls to getValue() and setValue() may be interleaved.

That is, no thread will be in getValue() at the same time another thread is in getValue() or setValue() and likewise no thread will be in setValue() while another thread is in getValue() or setValue().

However, there's no guarantee that a single thread will make sequential calls to setValue() getValue() without being preempted by another thread.

Basically, this is perfectly legal and possible:

Thread 1: v.setValue()
Some Other Thread(s): Any number of v.getValue()'s/v.setValue()'s
Thread 1: v.getValue()

Answer 4

This should get you the behavior you're looking for.

Thread 1:

class CongoThread implements Runnable {
    private ThreadValue v;

    public CongoThread(ThreadValue v) {
        this.v = v;
    }

    public void run() {
        for (int i = 0; i < 10; i++) {
            synchronized(v) {
                v.setValue( "congo", "cool" );
                System.out.println("In Congo Thread " + v.getValue() );
            }
        }
    }
}

Thread 2:

class LibyaThread implements Runnable {
    private ThreadValue v;

    public LibyaThread(ThreadValue v) {
        this.v = v;
    }

    public void run() {
        for (int i = 0; i < 10; i++) {
            synchronized(v) {
                v.setValue( "libya", "awesome" );
                System.out.println("In Libya Thread " + v.getValue() );
            }
        }
    }
}