user697911
user697911

Reputation: 10531

why is this thread safe?

Because it always prints out '3'. No synchronization needed? I am testing this simple thing because I am having a trouble in a real multiple thread problem, which isn't good to illustrate the problem, because it's large. This is a simplified version to showcase the situation.

class Test {

public static int count = 0;

class CountThread extends Thread {

    public void run()
    {
        count++;
    }
}  

public void add(){
    CountThread a = new CountThread();
    CountThread b = new CountThread();
    CountThread c = new CountThread();

    a.start();
    b.start();
    c.start();

    try {
        a.join();
        b.join();
        c.join();
    } catch (InterruptedException ex) {
        ex.printStackTrace();
    }

}
public static void main(String[] args) {

    Test test = new Test();
    System.out.println("START = " + Test.count);

    test.add();

    System.out.println("END: Account balance = " + Test.count);
}

Upvotes: 0

Views: 120

Answers (6)

xagyg
xagyg

Reputation: 9711

To make the class threadsafe either make count volatile to force memory fences between threads, or use AtomicInteger, or rewrite like this (my preference):

class CountThread extends Thread {

    private static final Object lock = new Object();

    public void run()
    {
        synchronized(lock) {
            count++;
        }
    }
}  

Upvotes: 0

danieln
danieln

Reputation: 4973

Seems to me like count++ is fast enough to finish until you invoke 'run' for the other class. So basically it runs sequential.
But, if this was a real life example, and two different threads were usingCountThread parallelly, then yes, you would have synchronization problem.

To verify that, you can try to print some test output before count++ and after, then you'll see if b.start() is invoking count++ before a.start() finished. Same for c.start().

Consider using AtomicInteger instead, which is way better than synchronizing when possible -

incrementAndGet
public final int incrementAndGet()
Atomically increments by one the current value.

Upvotes: 2

McDowell
McDowell

Reputation: 108859

This code is not thread-safe:

public static int count = 0;

class CountThread extends Thread {

    public void run()
    {
        count++;
    }
}

You can run this code a million times on one system and it might pass every time. This does not mean is it is thread-safe.

Consider a system where the value in count is copied to multiple processor caches. They all might be updated independently before something forces one of the caches to be copied back to main RAM. Consider that ++ is not an atomic operation. The order of reading and writing of count may cause data to be lost.

The correct way to implement this code (using Java 5 and above):

public static java.util.concurrent.atomic.AtomicInteger count =
                                 new java.util.concurrent.atomic.AtomicInteger();

class CountThread extends Thread {

    public void run()
    {
        count.incrementAndGet();
    }
}

Upvotes: 1

Gray
Gray

Reputation: 116828

Because it always prints out '3'. No synchronization needed?

It is not thread safe and you are just getting lucky. If you run this 1000 times, or on different architectures, you will see different output -- i.e. not 3.

I would suggest using AtomicInteger instead of a static field ++ which is not synchronized.

public static AtomicInteger count = new AtomicInteger();
...
public void run() {
    count.incrementAndGet();
}
...

Upvotes: 4

Alexei Levenkov
Alexei Levenkov

Reputation: 100527

It is not thread safe.

It just happened to be way to short to have measurable chance to show the issue. Consider counting to much higher number (1000000?) in run to increase chance of 2 operations on multiple threads to overlap.

Also make sure your machine is not single core CPU...

Upvotes: 0

Niels Keurentjes
Niels Keurentjes

Reputation: 41958

It's not thread safe just because the output is right. Creating a thread causes a lot of overhead on the OS side of things, and after that it's just to be expected that that single line of code will be done within a single timeslice. It's not thread safe by any means, just not enough potential conflicts to actually trigger one.

Upvotes: 0

Related Questions