3

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?

Nassim MOUALEK
  • 4,702
  • 4
  • 25
  • 44

3 Answers3

3

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 :)

Shubham Chaurasia
  • 2,472
  • 2
  • 15
  • 22
  • Thanks for the details – Nassim MOUALEK Aug 09 '15 at 14:04
  • You **think** that claim is true, but actually it's not, see [codegolf answer to *Write a program that makes 2 + 2 = 5*](http://codegolf.stackexchange.com/a/28818/29280) – fabian Aug 09 '15 at 14:06
  • 1
    From the perspective of thread safety, its a real thread safe code. And the reflection code which you (@fabian) mentioned above, reflection is neither meant for that nor its the good use of it. And I agree that you can always produce evils with reflection. But that is never recommended. – Shubham Chaurasia Aug 09 '15 at 14:22
2

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

ka4eli
  • 5,294
  • 3
  • 23
  • 43
2

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.

fabian
  • 80,457
  • 12
  • 86
  • 114
  • So we could say it safe unless we mess with Reflection? – Nassim MOUALEK Aug 09 '15 at 14:38
  • @NassimMOUALEK, it's not about "messing with reflection." Fabian's example is subverting the `Integer` class so that Integer objects will have different values from what you normally would expect. That is to say, he's hacking it to return _wrong_ values, and then he's saying, Watch Out because his hack won't necessarily produce the _same_ wrong values in different threads. "Reflection" is not the problem. The problem is what he's using reflection _for_. – Solomon Slow Aug 09 '15 at 18:04
  • OK, this question is a recruter question, i am just asking what is the best answer, we couldn't say no its not thread safe, the good response, is yes it is unless we mess with reflection – Nassim MOUALEK Aug 09 '15 at 18:08