croraf
croraf

Reputation: 4720

Threads - simple concurrency issue

Ok this is the problem: the following code is obviously not written thread safe, cause the method increment() is not syncronized. The output varies, and doesn't go like: 1,2,3,4,5,6...

For example the output can be this: 1,2,3,4,5,1... but then it continues like this 6,7,8,... and reaches 100. I don't understand how can it reach 100, shouldnt after second 1 come 2 again, at least in some cases, couse the balance was wrongly updated by the other thread. The question is this: why after 1 it continues normally with 6, 7...

class Job implements Runnable{
private int balance;

public void run(){
    for (int i = 0; i < 50; i++) {
        increment();
        System.out.println(balance);
        int a = 3;
        int b = 4;
        a = b;
    }

}

public void increment(){
    int i = balance;
    balance = i+1;
}
}

public class ThreadsDemo{
public static void main(String[] args) {
    Job job = new Job();

    Thread alpha = new Thread(job);
    Thread beta = new Thread(job);

    alpha.setName("alpha");
    beta.setName("beta");

    alpha.start();
    beta.start();

}
}

To explain further, here is one of the possible outcomes:

Thread 1: balance - 0

i - 0 (thread 1 put back to runnable)

Thread 2:
balance - 0, 1, 2, 3, 4, 5

i - 0, 1, 2, 3, 4, (thread 2 put back to runnable)

Thread 1 (put to running): balance - ... 1, 2

i - 0, 1

(In some cases it can update normaly, but in 50 iterations it must come some abnormal update) How does every outcome reach 100, is this some IDE optimization to deal with thread interleaving or what?


ANSWER: So no need for latch in this example, just that the thread was "blocking" on print, and the other could finish the update in the meantime. Ty Affe

class Job implements Runnable{
public int balance = 0;
//public static CountDownLatch latch = new CountDownLatch(1);

public void run(){

        for (int i = 0; i < 50000; i++) {
            increment();
        }
}

public void increment(){
    int i = balance;
    balance = i+1;
}
}

public class ThreadsDemo{
public static void main(String[] args) {
    Job job = new Job();

    Thread alpha = new Thread(job);
    Thread beta = new Thread(job);

    alpha.setName("alpha");
    beta.setName("beta");

    alpha.start();
    beta.start();

    try {
        alpha.join();
                    beta.join();
    } catch (Exception ex) { }
    System.out.println(job.balance +"   "+ alpha.isAlive() + "    " + beta.isAlive());
}
}

The output is around 60 000, as it was expected.

Upvotes: 0

Views: 101

Answers (1)

Affe
Affe

Reputation: 47954

You calculation is very fast and starting threads is very slow. The first one is completely done before the second one ever begins. The out-of-place numbers in the output are likely just issues with buffer flushing in the operating system.

Add a latch so that both threads actually start at the same time, and use a sufficiently large number, and you will see a total that doesn't add up.

public static CountDownLatch latch = new CountDownLatch(1);
private static class Job implements Runnable{
private int balance;

public void run(){
    try {
    latch.await();
    } catch (InterruptedException e) {}
    for (int i = 0; i < 50000; i++) {
        //existing code
    }
}
public void increment(){
    int i = balance;
    //make it work harder so there's more opportunity for an actual interleave
    balance = new BigInteger(Integer.toString(i)).add(BigInteger.ONE).intValue();
}
}

public static void main(String[] args) {
    //existing code
    alpha.start();
    beta.start();
    try {
        Thread.sleep(100);
    } catch (InterruptedException e) {}
    latch.countDown();

}

Upvotes: 1

Related Questions