CODEWITHSUNDEEP
javaconcurrency
user2045279
user2045279

Reputation: 711

Java synchronized confusion

sorry if this is extremely obvious or has been answered elsewhere. I haven't been able to find anything. I have the following code:

public class SimpleThread extends Thread {
    public static Integer sharedVal = 0;
    public SimpleThread() {

    }
    @Override
    public void run() {
       while(true) {           
           iterator();
       }
    }

    public void theSleeper() {
        System.out.println("Thread: " + this.getId() + " is going to sleep!");
        try {
            this.sleep(5000);
        } catch(Exception e) {

        }
    }

    public void iterator() {
        synchronized(sharedVal)  {
            System.out.println("Iterating sharedVal in thread: " + this.getId());
            sharedVal++;
            System.out.println(sharedVal);
            theSleeper();
            System.out.println("Thread : " + getId() + " is done sleeping, trying to iterate again..."); 
        }
    }
}

I create two instances of this SimpleThread class, and execute the run methods. I would expect to see something like: Thread 9 incrementing... Thread 9 sleeping... (after 5 seconds) Thread 10 incrementing.... Thread 10 sleeping... . I expect this because I am locking the iterator method so that only one thread at a time should be able to enter it. What happens instead is that both threads increment, and then both wait 5 seconds. This repeats forever. What am I missing here such that I would get the expected behavior? Thanks very much!

EDIT: I made a new public static variable: public static Object theLock = new Object(). Now, in the iterator method, I do synchronized(theLock). The output is now more as I expected, because the lock never changes. However, now only thread 9 ever enters the method. It seems that thread 10 is starving, and never gets a turn. This seems strange. It's not just a few times, its always just thread 9 iterating and sleeping. I would think that it would be 9, 10, 9, 10. Or perhaps a random distribution like 9, 10, 10, 10, 9, 10, 9, 9, 10, etc.

EDIT2: I see what's happening now. Thread 9 has the lock. Thread 10 tries to enter the function, but is immedietly told to wait. After the function completes, it may still be Thread 9s turn. Thread 9 then reaqquires the lock, and the cycle continues. The timing window for thread 10 to get a turn is very small, and if it did get a turn it would likely starve 9. That being said, putting yield() after the synchronized block in iterator() does not seem to make it more fair. I read the comments on the method, and the scheduler can in fact ignore yield().

Upvotes: 6

Views: 256

Answers (4)

Darkhogg
Darkhogg

Reputation: 14165

Your problem resides here:

sharedVal++;

That line, due to autoboxing translates into this:

sharedVal = Integer.valueOf(sharedVal.intValue() + 1);

which creates a new Integer object every time it runs, so every time the synchronized block isreached, it locks a different value.

Use a dedicated object to synchronize:

private final static Object LOCK = new Object();

and then change your synchronization to use synchronized (LOCK) {... instead.

(You could also use getClass() to lock, but I personally don't like to expose lock objects to the public world)

Upvotes: 3

Nikoloz
Nikoloz

Reputation: 553

sharedVal member varible instance is changing when you perform sharedVal++. Instead you can use for synchronization the following statement:

synchronized(SympleThread.class)

Upvotes: 0

Sami Korhonen
Sami Korhonen

Reputation: 1303

You are effectively changing the lock that threads are using on every iteration: ++ on Integer creates a new instance (Integer class is immutable)

Upvotes: 1

anstarovoyt
anstarovoyt

Reputation: 8136

When you are doing increment you create a new instance of integer and a new object for locking.

Upvotes: 5

Related Questions