Thread safe read/write to a counter

Viewed 1332

Im trying to make 2 threads that read/write to a counter using thread safe methods.

I have written some code to try test this but the read thread just reads the counter at its max (1000)

Main:

public static void main(String[] args) {

    Counter c = new Counter();

    Thread inc = new Increment(c);
    Thread read = new Read(c);

    inc.start();
    read.start();

}

Counter:

public class Counter {

private int count;

public Counter() {
    count = 0;
}

public synchronized void increment() {
    count++;
}

public synchronized int getVal() {
    return count;
}

}

Increment:

public class Increment extends Thread {

private static final int MAX = 1000;
private Counter myCounter;

public Increment(Counter c) {
    myCounter = c;
}

public void run() {
    for (int i = 0; i < MAX; i++) {
        myCounter.increment();
    }
}
}

Read:

public class Read extends Thread {

private static final int MAX = 1000;
private Counter myCounter;

public Read(Counter c) {
    myCounter = c;
}

public void run() {
    for (int i = 0; i < MAX; i++) {
        System.out.println(myCounter.getVal());
    }
}
}

Would I be better off using Atomic Integer to hold the value of the counter to allow me to safely increment it and get the value?

2 Answers

Your code is perfectly fine as is. It just so happened that your increment thread finished all its increments before the read thread got a chance to read. 1,000 increments takes almost no time at all.

If you want interleave execution of Read thread and Increment thread much more often then the natural operating system thread pre-emption, just make each thread give up their lock (by calling <lockedObject>.wait() followed by <lockedObject>.notify() or notifyAll() in the respective run() methods:

[In Reader]:

public void run() {
    for (int i = 0; i < MAX; i++) {
        synchronized (myCounter) {
            System.out.println(myCounter.getVal());
            try {
                myCounter.wait(0L, 1);
                myCounter.notifyAll();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

[In Increment]:

public void run() {
    for (int i = 0; i < MAX; i++) {
        synchronized (myCounter) {
            myCounter.increment();
            try {
                myCounter.wait(0L, 1);
                myCounter.notifyAll();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

Upping the MAX constant to 1_000_000_000 (1 billion) made the treads interleave as well every now and then (on my machine interleave happened just by gazing at few printouts between 150 and 400_000 iterations).

Related