Java multithreading not thread-safe using synchronized

Question

A simple multithreading test with synchronization. I thought if it was "synchronized," other threads would wait. What am I missing?

public class MultithreadingCounter implements Runnable {

    static int count = 0;

    public static void main(String[] args) {
        int numThreads = 4;
        Thread[] threads = new Thread[numThreads];

        for (int i = 0; i < numThreads; i++)
            threads[i] = new Thread(new MultithreadingCounter(), i + "");

        for (int i = 0; i < numThreads; i++)
            threads[i].start();

        for (int i = 0; i < numThreads; i++)
            try {
                threads[i].join();
            } catch (Exception e) {
                e.printStackTrace();
            }
    }           

    @Override
    public void run() {
        increment();
    }

    public synchronized void increment(){
            System.out.print(Thread.currentThread().getName() + ": " + count + "\t");
            count++; // if I put this first or increment it directly in the print line, it works fine.
    }
}

I thought this would display something like:

0: 1    2: 0    1: 2    3: 3    

But its actual output:

0: 0    2: 0    1: 0    3: 3    

and other variations like this. It should display each increment (i.e. 0,1,2,3) not in order...

Answer 1

Your synchronized keyword is on an instance method. No two threads can execute this method of one of your thread objects at the same time. But, that is not what your code does. Each thread executes the method on its own instance. The synchronization does not do what you seem to intend. If it were a static method, it would.

Answer 2

Your increment method should be static:

public static synchronized void increment() {

Right now, each object is synchronized on that individual instance, but since count is a static variable, you should be synchronizing on the Class object itself.