CODEWITHSUNDEEP
javamultithreadingthread-safetyatomic
John Anderson
John Anderson

Reputation: 33

Thread safe read/write to a counter

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?

Upvotes: 3

Views: 2362

Answers (2)

diginoise
diginoise

Reputation: 7620

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).

Upvotes: 0

David Schwartz
David Schwartz

Reputation: 182819

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.

Upvotes: 3

Related Questions