Multiple threads accessing inner class

Question

I have created 3 threads which are accessing inner class MyInnerClass of ThreadsAroundInnerClasses outer class.

package com.test;

public class ThreadsAroundInnerClasses {
    public static void main(String[] args) {
        Thread t1 = new Thread(new MyThread(), "THREAD-1");
        Thread t2 = new Thread(new MyThread(), "THREAD-2");
        Thread t3 = new Thread(new MyThread(), "THREAD-3");
        t1.start();
        t2.start();
        t3.start();
    }

    static class MyInnerClass {
        static int counter = 0;
        public void printIt(String threadName) {
            System.out.println("I am inside inner class, counter value is " + ++counter + " and thread name is " + threadName);
        }
    }
}

class MyThread implements Runnable {
    @Override
    public void run() {
        ThreadsAroundInnerClasses.MyInnerClass innerObj = new ThreadsAroundInnerClasses.MyInnerClass();
        innerObj.printIt(Thread.currentThread().getName());
    }
}

In the output I can see that counter static variable in MyInnerClass class is not getting updated in sequential order.

I am inside inner class, counter value is 1 and thread name is THREAD-1
I am inside inner class, counter value is 3 and thread name is THREAD-2
I am inside inner class, counter value is 2 and thread name is THREAD-3

It would be of great help if someone can explain how inner classes are handle in case of multithreading? Can we synchronize whole inner class?

Thanks in advance for the help.

Answer 1

Whether the class is an inner class or not is irrelevant. What you see here is expected output: you don't have any synchronization in your program, and the thread scheduler is thus free to switch to other thread when it wants to. Here's a sequence of operations that is possible and that would lead to the output you're seeing.

1. T1 : increment counter
2. T1 : concaenate
3. T1 : print
4. T3 : increment counter
5. T3 : concatenate
6. T2 : increment counter
7. T2 : concatenate
8. T2 : print
9. T3 : print

If you want the counter incrementation, the concatenation and the print to be an atomic operation, then you should make sure it's synchronized on a unique lock:

synchronized MyInnerClass.class {
    System.out.println("I am inside inner class, counter value is " + ++counter + " and thread name is " + threadName);
}

Answer 2

Try this:

    static class MyInnerClass {
    static int counter = 0;
    public void printIt(String threadName) {
        synchronized(MyInnerClass.class) {
            System.out.println("I am inside inner class, counter value is " + ++counter + " and thread name is " + threadName);
        }
    }
}

Your counter is static, so you need to synchronize on the whole class object.

Answer 3

This is a question of read-then-change and of thread visibility

Read-then-change

An incrementation is first a read, then an addition, and then a write back. ++counter is short hand for counter = counter + 1 so that means: "read counter, add one, write the result back to counter". This flow can be interrupted in the middle by another thread: Thread1 reads counter, gets 1 - and gets interrupted. Thread2 reads counter - it's still 1 - adds 1 - and gets interrupted. Thread3 reads counter - it's still 1 - .... and so on. The behaviour is basically random. You can probably provoke different output just by running other applications in the background.

Thread visibility

For a non-volatile, non-synchronized variable on a multi-core architecture, different threads may be running on different cores, and the cores may cache the variable on their registers. This means that one thread may not be able to see what another thread has written to the variable - until it is committed back to memory and there is a new write.

Synchronization is one way of creating memory barriers. The system will guarantee that everything that was written within the lock is visible when the lock is taken the next time. In effect - when you exit the synchronized block, everything you did is committed to memory. Then you enter the synchronized block, everything is read back from memory.

The volatile keyword tells the system it is forbidden to cache the variable in a register - it must be read and written in memory always. This makes everyone read something that is up to date, but it doesn't solve your problem, for reasons described above.

The easy solution

The easiest way to solve your specific problem is to use the AtomicInteger.

static class MyInnerClass {
    static final AtomicInteger counter = new AtomicInteger(0);
    public void printIt(String threadName) {
        System.out.println("I am inside inner class, counter value is " +
            counter.incrementAndGet() + " and thread name is " + threadName);
    }
}

This will save you all the trouble of synchronization - it is contained within the AtomicInteger class.