CODEWITHSUNDEEP
javamultithreading
Nassim MOUALEK
Nassim MOUALEK

Reputation: 4864

Simple add method Thread safe?

Having this simple class, with addition method :

class A {
   public Integer add (int a, int b){
     return a+b;
   }
}

is it thread safe or not..? it looks safe for me, but most poeple answer no, could anyone explain why?

Upvotes: 3

Views: 381

Answers (3)

fabian
fabian

Reputation: 82461

Actually that method is not thread safe, but it requires you to know a bit about the internals of the Integer class to understand why. Let's look at some code that yields the same bytecode:

class A {
   public Integer add (int a, int b){
     // auto boxing hidden in OP's implementation
     return Integer.valueOf(a+b);
   }
}

For small enough values the Integers are cached and looked up in a array. Using reflection you can access that array and change it's elements. Those changes are not synchronized, therefore if you change those elements, from another thread the result of your method can change too.

The following code should demonstrate the problem on most java VMs: There is a race condition in your method. In most cases it will print 4s and 5s:

import java.lang.reflect.Field;

class A {

    public Integer add(int a, int b) {
        return a + b;
    }

    private static volatile boolean cont = true;

    public static void main(String[] args) throws NoSuchFieldException, IllegalArgumentException, IllegalAccessException, InterruptedException {
        final A a = new A();

        new Thread(() -> {
            while(cont) {
                for (int i = 0; i < 100; i++) {
                    // print result of add method
                    System.out.println(a.add(2,2));
                }
            }
        }).start();

        // give other thread time to start
        Thread.sleep(1);

        // mess around with the internals of Integer
        Class cache = Integer.class.getDeclaredClasses()[0];
        Field c = cache.getDeclaredField("cache");
        c.setAccessible(true);
        Integer[] array = (Integer[]) c.get(cache);
        array[132] = array[133];

        cont = false;
    }
}

However in most cases nobody messes around with the internals of Integer. If the array in the Integer class is never modified, the values wrapped by the Integer objects returned by your method will always be correct, since the shared state used by Integer.valueOf is never modified. Therefore it would be thread-safe in that case.

Upvotes: 2

Shubham Chaurasia
Shubham Chaurasia

Reputation: 2632

Thread safety should be bothered about only when you have some means of sharing state and you modify that without any locks or synchronization i.e. you modify a shared variable(class level variable) then only you should care about thread safety.
Here there is no issue of thread safety.
And in this particular case each variable is local and that location will not be shared by threads as each function call will have their on separate allocation on stack along with their local variables you should not bother anyways :)

Upvotes: 3

ka4eli
ka4eli

Reputation: 5424

It is completely thread safe, because all variables are local.

Upvotes: 2

Related Questions