3
public class Test{
private First first;
private Second second;

public void setFirst(First first){
this.first = first;
}
public First getFirst(){
 return first;
}
// same approach for second
}

If I inject the instance through spring injection, is it thread-safe? If not, how to make it thread-safe? I googled and found contradict responses, not able to come to any conclusion.Please suggest. Thanks in Advance. -RW

rock wells
  • 51
  • 2
  • 5
  • 8
    Your question is too vague. What do you mean? Give better examples, and why you think it might not be thread-safe. – skaffman Feb 15 '11 at 10:58
  • Please clarify your question. The class you showed is always thread-safe, since it doesn't allow any access to its state (except for dirty hacks with `setAccessible()`). – axtavt Feb 15 '11 at 10:59

3 Answers3

7

If you're talking about what I think you're talking about, then it's an interesting discussion.

Technically, because setFirst() and getFirst() are not synchronized, then it's possible for setFirst() to inject one object on Thread 1, and getFirst() to return a different object on Thread 2. The Java memory model reserves the right to make this "eventually consistent", as they say.

So in the case of Spring, which configures its bean graph during startup (or the server's internal startup thread), it's possible that the HTTP request threads (for example) would not see that bean graph properly, due to the lack of synchronization.

Note: this has nothing to do with concurrent access. Even if the HTTP requests come in after Spring has initialized, the Java memory model does not guarantee that those threads will see the correct data.

In practice, this never happens (at least, I've never seen it happen). The lack of synchronization is only really a problem for concurrent threads, which isn't the issue here.

So it's really just an academic argument.

If this isn't what you're talking about, my apologies. It's still an interesting thought, though.

skaffman
  • 398,947
  • 96
  • 818
  • 769
  • This class will be used by concurrrent request (ie, multi thread). Do we need to take care about this? I googled and found this link, http://blog.webscale.co.in/?p=64 . Any suggestion? – rock wells Feb 15 '11 at 11:57
  • 1
    A quote from Java Concurrency in Practice, "If multiple threads access the same mutable state variable without appropriate synchronization, your program is broken". So the class must either be thread safe or be used in a thread safe way irrespective of how it was instantiated / injected. – Carl Pritchett Nov 21 '12 at 00:17
0

The injection of the dependencies is handled by Spring, you should not care about the thread safety of the getters and setters, because Spring is going to inject the dependencies taking in account that you have no synchronization logic.

Also, no other thread (i.e. the threads of your application) is going to use the beans until the context is completely configured and all the dependencies have been injected.

You should not worry about thread safety of the dependency injection, Spring does (most likely, spring is going to use just one thread to inject all the dependencies).

Mr.Eddart
  • 10,050
  • 13
  • 49
  • 77
  • 5
    I don't think this is entirely correct. Spring (probably) only uses one thread to construct its bean graph - so this part is safe. However, this doesn't mean that everything will be visible to another thread afterwards (memory visibility). So you have to care about thread safety - Spring doesn't take care of that. – Markus Kramer Jun 21 '12 at 09:53
-1

Assuming that you have created singleton instance of Test and trying to set/get value (a) from Spring (b) from another part of you program. In this case just make your get/set pair synchronized:

public synchronized void setFirst(First first){
    this.first = first;
}
public synchronized First getFirst(){
    return first;
}

I don't see other unsafety from multithreading

Dewfy
  • 23,277
  • 13
  • 73
  • 121
  • 1
    Synchronizing your methods just like that is the worst thing you can do. First, you are not protecting your this.first in any way, you are just making sure that only one thread at a time can go into setFirst and getFirst methods, which is pointless. – Spajus Feb 15 '11 at 11:27
  • 1
    @Spajus - read attentively my precondition. Using `synchronize(this)` ensures that during reading value of this.first it wouldn't be modified. According to java spec assignment is not granted to be atomic operation. So for some platform there is possibility that some bytes of `this.first` are modified and some aren't. (For example you can google term 'broken pattern' that is broken in java exactly because of non-atomic of assignment) – Dewfy Feb 15 '11 at 11:36
  • @spajus- This class will be used by concurrrent request (ie, multi thread). Do we need to take care about this? I googled and found this link, [url](blog.webscale.co.in/?p=64) . Any suggestion? – rock wells Feb 16 '11 at 07:19
  • @rock wells - I'm not @spajus, but know the answer. Creation of ben in specified article shows very exotic problem when you in concurrent way trying setup singleton from code and from Spring context. Usually it is exotic, because you if you do so, then why do you need Spring at all? In correct architecture and correct Spring usage creation of bean can be unsafe without damaging any logic. But usage of these beans instances should be synchronized, so regardless of (-2) I persist, that get/set should be secured. – Dewfy Feb 17 '11 at 12:43