Is this thread safe? Simple question.

[Bryan Lee]

Hi,
Just wondering if this is thread safe:

class aThread extends Thread{
    private static int aNumber = 1;
 
    public void run(){
        :
        System.out.println(aNumber++);
        :
    }
}

I know I can have a synchronized method like this to ensure thread safety:

    private synchronized int getNumber(){
        return aNumber++;
    }

Just curious whether a static variable can ensure the thread safety?

[Ulf Dittmer]

If the field is private, no other object can get at it, so it's safe. Making it static makes it thread-unsafe, because other objects of the same class can get at it via aThread.aNumber.

[Bryan Lee]

Anyone can recommend me a tutorial in multi-threading, thread-safety...etc. areas?

[John Smith]

If the field is private, no other object can get at it, so it's safe.

That's not quite right. The private field can be (and most likely is) modified from a public method (or from a private method that is called from a public method).

Making it static makes it thread-unsafe, because other objects of the same class can get at it via aThread.aNumber

That is also not quite right. The fact is, the getNumber() method above is unsafe no matter if the aNumber field is static or not. Here is some code to demonstrate the idea:

class SafetyCandidate {
    private int number;
    // the highly suspect method in question
    public int getNumber() {
        return number++;
    }
}
 
class T extends Thread {
    private final SafetyCandidate sc;
    private final ThreadTest tt;
    private final int iterations;
 
    public T(SafetyCandidate sc, ThreadTest tt, int iterations) {
        this.sc = sc;
        this.tt = tt;
        this.iterations = iterations;
        start();
    }
 
    public void run() {
        for (int i = 0; i < iterations; i++) {
            sc.getNumber();
        }
        tt.signalComplete();
    }
}
 
public class ThreadTest {
    private static final int TOTAL_THREADS = 100;
    private static final int ITERATIONS = 1000000;
    private static int activeThreadCount = TOTAL_THREADS;
 
    synchronized public void signalComplete() {
        activeThreadCount--;
        notifyAll();
    }
 
    synchronized public void waitUntilAllDone() throws InterruptedException {
        while (activeThreadCount != 0) {
            wait();
        }
    }
 
    public static void main(String[] s) throws InterruptedException {
        ThreadTest tt = new ThreadTest();
        SafetyCandidate sc = new SafetyCandidate();
        for (int i = 0; i < TOTAL_THREADS; i++) {
            new T(sc, tt, ITERATIONS);
        }
 
        tt.waitUntilAllDone();
        System.out.println("Expected: " + (TOTAL_THREADS * ITERATIONS)
                           + " Actual: " + sc.getNumber());
    }
}

The output is likely to be different every time you run this test. Here is what I got from two separate runs:

run1: Expected: 100000000 Actual: 45381942
run2: Expected: 100000000 Actual: 72689195
[ June 30, 2005: Message edited by: John Smith ]

[Jack Wiesenthaler]

If you want to store data on a per thread basis then use ThreadLocal variables ;-)

private static ThreadLocal aNumber = new ThreadLocal();

[Jaspreet Singh]

How can you make this code threadsafe, and get an exact value of 100000000 ?

class SafetyCandidate {
    private int number;
    // the highly suspect method in question
    public int getNumber() {
        return number++;
    }
}
 
class T extends Thread {
    private final SafetyCandidate sc;
    private final ThreadTest tt;
    private final int iterations;
 
    public T(SafetyCandidate sc, ThreadTest tt, int iterations) {
        this.sc = sc;
        this.tt = tt;
        this.iterations = iterations;
        start();
    }
 
    public void run() {
        for (int i = 0; i < iterations; i++) {
            sc.getNumber();
        }
        tt.signalComplete();
    }
}
 
public class ThreadTest {
    private static final int TOTAL_THREADS = 100;
    private static final int ITERATIONS = 1000000;
    private static int activeThreadCount = TOTAL_THREADS;
 
    synchronized public void signalComplete() {
        activeThreadCount--;
        notifyAll();
    }
 
    synchronized public void waitUntilAllDone() throws InterruptedException {
        while (activeThreadCount != 0) {
            wait();
        }
    }
 
    public static void main(String[] s) throws InterruptedException {
        ThreadTest tt = new ThreadTest();
        SafetyCandidate sc = new SafetyCandidate();
        for (int i = 0; i < TOTAL_THREADS; i++) {
            new T(sc, tt, ITERATIONS);
        }
 
        tt.waitUntilAllDone();
        System.out.println("Expected: " + (TOTAL_THREADS * ITERATIONS)
                           + " Actual: " + sc.getNumber());
    }
}

The output is likely to be different every time you run this test. Here is what I got from two separate runs:

run1: Expected: 100000000 Actual: 45381942
run2: Expected: 100000000 Actual: 72689195

[ June 30, 2005: Message edited by: John Smith ][/QB]

[Craig Jackson]

How can you make this code threadsafe, and get an exact value of 100000000 ?

You can synchronize the getNumber() method:

public synchronized int getNumber() {        
    return number++;    
}