Synchronized doesn't help to achieve mutual exclusion

Question

Class C:

class C extends Thread
{
    public static int cr;

    C(int n) 
    {
        cr = n;
    }

    public void run()
    {
        go();
    }

    synchronized void go()
    {
        for (int i = 0; i < 10000; i++)
        {
            cr++;
        }
    }
}

Class Launch

class Launch
{
    public static void main(String args[]) throws InterruptedException
    {
        C[] c = new C[10];

        for (int i = 0; i < 10; i++)
        {
            c[i] = new C(0);
        }

        for (int i = 0; i < 10; i++)
        {
            c[i].start();
        }

        System.out.println(C.spaces);
    }
}

It doesn't give me 100,000, but rather numbers below 100k. Why? I made method go() synchronized, so it should be used by only one thread at a time..? What am I missing?

Answer 1

synchronized void go(){...} means that it is synchronized on current instance (on this). Since this method belongs to your custom Thread class and you are crating 10 threads there exist 10 different this references.

That is one of the reasons it is preferred to not extend Thread class, but to implement Runnable interface and pass instance of this interface to as many threads as you want.

Another problem is that you are printing edited value without waiting for threads to finish.

What you need is creating one instance which will hold value you want to change and invoke synchronized method from only one instance, because as mentioned by default synchronized void method is synchronized on this (current object on which this method is invoked).

class MyTask implements Runnable {

    public volatile int counter;

    MyTask(int n) {
        counter = n;
    }

    public void run() {
        System.out.println(Thread.currentThread().getName()+" entered run");
        go();
        System.out.println(Thread.currentThread().getName()+" finished run");
    }

    synchronized void go() {

        System.out.println(Thread.currentThread().getName()+" entered go");
        for (int i = 0; i < 10000; i++) {
            counter++;
        }
        System.out.println(Thread.currentThread().getName()+" left from go");
    }
}

class Luncher {
    public static void main(String args[]) throws InterruptedException {

        //lets create task we want to execute in parallel
        MyTask task = new MyTask(0);

        Thread[] threads = new Thread[10];
        for (int i = 0; i < 10; i++)//create thread instances
            threads[i] = new Thread(task);

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

        for (int i = 0; i < 10; i++)
            threads[i].join();//hold main thread to wait till all threads will finish
        System.out.println(task.counter);
    }
}

Answer 2

with

 c[i] = new C(0);

you are creating new instance of c every time

with

 synchronized void go()
{
    for (int i = 0; i < 10000; i++)
    {
        cr++;
    }
}

you will always get the number less than 10000 i dont know why you are expecting 100,000

Is it a typo mistake i < 10000 where you should have u used 100,000 ?

To answer your last question

I made method go() synchronized, so it should be used by only one thread at a time..?

if only one thread at a time, you don't need synchronized(multithreading concept).

Answer 3

It doesn't give me 100,000, but rather numbers below 100k. Why?

Every thing is working fine but main thread is not waiting for other threads to complete the calculation hence output is random in main thread.

Use Thread#join() so that main thread waits for all the other threads to die before executing the last line of the main thread.

for (int i = 0; i < 10; i++) {
    c[i].start();
    c[i].join(); // Waits for this thread to die
}

System.out.println(C.cr); // output 100000

---

It's worth reading How do I pause main() until all other threads have died?