Basic Concurrency code example Java

Question

I am reading concurrency from Thinking in Java, 4th edition by Bruce Eckel. Here is a basic example code from the book to demonstrate the need of synchronization.

//SerialNumberGenerator.java

    public class SerialNumberGenerator {
        private static volatile int serialNumber = 0;
        public static int nextSerialNumber() {
            return serialNumber++; // Not thread-safe
        }
    }


//: concurrency/SerialNumberChecker.java
// Operations that may seem safe are not,
// when threads are present.

import java.util.concurrent.*;
// Reuses storage so we don’t run out of memory:
class CircularSet {
    private int[] array;
    private int len;
    private int index = 0;
    public CircularSet(int size) {
        array = new int[size];
        len = size;
// Initialize to a value not produced
// by the SerialNumberGenerator:
        for(int i = 0; i < size; i++)
            array[i] = -1;
    }
    public synchronized void add(int i) {
        array[index] = i;
// Wrap index and write over old elements:
        index = ++index % len;
    }
    public synchronized boolean contains(int val) {
        for(int i = 0; i < len; i++)
            if(array[i] == val) return true;
        return false;
    }
}
public class SerialNumberChecker {
    private static final int SIZE = 10;
    private static CircularSet serials =
            new CircularSet(1000);
    private static ExecutorService exec =
            Executors.newCachedThreadPool();
    static class SerialChecker implements Runnable {
        public void run() {
            while(true) {
                int serial =
                        SerialNumberGenerator.nextSerialNumber();
                if(serials.contains(serial)) {
                    System.out.println("Duplicate: " + serial);
                    System.exit(0);
                }
                serials.add(serial);
            }
        }
    }
    public static void main(String[] args) throws Exception {
        for(int i = 0; i < SIZE; i++)
            exec.execute(new SerialChecker());
        // Stop after n seconds if there’s an argument:
        if(args.length > 0) {
            TimeUnit.SECONDS.sleep(new Integer(args[0]));
            System.out.println("No duplicates detected");
            System.exit(0);
        }
    }
}

The output he has mentioned in the book is any number like this:

Duplicate: 8468656

When I ran the code, I got the output:

Duplicate: 3484
Duplicate: 3485

I know the program is thread unsafe and that the numbers could be different but, why am I getting 2 duplicate consecutive values here? How is this possible?
Can anyone explain(the low level details) the process of Duplicate number generation in the above program?

Answer 1

try to use:

public static synchronized int nextSerialNumber()
{
    return serialNumber++;
}

this 'synchronized' solve your problem but you should use java.util.concurrent.atomic.AtomicInteger - better solution.

Answer 2

Okay, Holger's answer is okay for explaining 2 duplicates and on the whole he has explained about Thread unsafe code leading to various indeterminate possibilities. I agree with that. However, I am posting low level details that could help in explaining one of those many possibilities due to which above code might have arisen.

javap -c SerialNumberGenerator.class

Compiled from "SerialNumberGenerator.java"

public class SerialNumberGenerator {
  public SerialNumberGenerator();
    Code:
       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":()V
       4: return

  public static int nextSerialNumber();
    Code:
       0: getstatic     #2                  // Field serialNumber:I
       3: dup
       4: iconst_1
       5: iadd
       6: putstatic     #2                  // Field serialNumber:I
       9: ireturn

  static {};
    Code:
       0: iconst_0
       1: putstatic     #2                  // Field serialNumber:I
       4: return
}

As you can see here, there is an instruction between iadd and ireturn. Its possible that, while one thread loaded the value of serialNumber and blocked another would have loaded it too and when one of them increments it and adds it to the queue. Other thread holding the unincremented value now increments it to get the same value and a check against the array returns "Duplicate". Then as, Holger said,

The statements

System.out.println("Duplicate: " + serial);
System.exit(0);

do not prevent other threads from performing actions in-between. Its possible that one thread got blocked before got executing the exit(after printing the println statement) and other one also executed the prinln statement. When the scheduler got back to the first thread, it exited. This are just a typical possibility from many others. The intent is to visualize the process that could have led to the output so that the picture becomes more clear.

Answer 3

It comes from the SerialNumberGenerator. Replace the int value by an instance of AtomicInteger and you should never get duplicates.

Ah.. What Holger wrote is right.

Answer 4

The statements

System.out.println("Duplicate: " + serial);
System.exit(0);

do not prevent other threads from performing actions in-between. Therefore, if you run n threads, all calling the unsafe code, thus potentially executing these two statements, there might be up to n threads printing their message before one of these threads manages to execute System.exit(0);