File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Threads and Synchronization and the fly likes Is this method thread safe? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Soft Skills this week in the Jobs Discussion forum!
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Is this method thread safe?" Watch "Is this method thread safe?" New topic
Author

Is this method thread safe?

Mick Peterson
Greenhorn

Joined: Oct 17, 2012
Posts: 5
Hello,

First post.

'ThreadShared' instance is shared by several threads. Each thread calls the method 'createResultObj()' with different object parameters.
My question is: is createResultObj() thread safe, in such a way that objX within the method will always point to the same instance within the lifespan of the method, even though the will be called by several threads at the same time?



I look forward to any help/explanation on the matter!
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

In answer to the questions which you asked: local variables are local to the thread which creates them, so they can't be affected by other threads. In particular since other threads don't have access to them, they can't assign values to them.

So it's the class-level variables you should be concerned about. Like objXMap, for example. And in answer to the question you didn't ask, you don't know that its remove() method is thread-safe. All you know is that it references a Map object, but you don't know whether it's (for example) a ConcurrentMap, which guarantees that remove() is an atomic operation or just any old Map, which doesn't guarantee that.

And if method A calls method B which isn't thread-safe, that makes A be not thread-safe as well. You have two more instances of this situation to check, down at the bottom of your code.
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

And welcome to the Ranch, Mick!
Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

Paul Clapham wrote:In answer to the questions which you asked: local variables are local to the thread which creates them, so they can't be affected by other threads. In particular since other threads don't have access to them, they can't assign values to them


It's worth noting, however, that local variables can refer to objects that may be accessible from other threads, so we can't just assume a method is thread-safe because it uses only locals.

I haven't looked at this code, so I don't know if it's the case here, but it's something to be aware of.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

The instance that the variable objX points to will not change during the course of the method call, but there is no guarantee that the data will be in a consistent state.

For example, there would be no guarantee that objXMap.remove(name) will yield an Object, and not null, even if just before this method call you put an ObjX into the map with the correct name. this is not guaranteed because some other thread could be manipulating the map at the same time and remove the object you are trying to work with.

You get two different values from objX. We don't know what objX is, or how safe it is to use. It could be that during the call of this method its data changes such that when you get a and b out of it, those values are inconsistent because the instance of objX has been manipulated.

Because you are using the service object, and that service object is being used by different threads, giving it two consistent values as input may produce inconsistent output because the internal data of service may change during the execution of the doSomething() method.

Finally, you are adding the results to a List which is also shared among threads. Two threads could be trying to add to the same index, and only one value will get stored. So there is a chance of getting missing or mutilated results.

So we can't say for sure that the method will break in multi-threaded environments, we just don't have enough information. We can say that you can't be sure it is safe. You definitely need some protections to be sure:
1) ObjX should probably be immutable, as should SomeObj
2) You need synchronization around all parts that require consistency
3) Almost everything in this method can be modified in the outside world. You pass the objXMap in from an external source. The external source could be doing whatever it wants to that map, so no amount of synchronization in this method (or even this class) is enough to ensure safety.
4) Similarly the service, and the ObjX inside the map (and maybe the SomeObj). These come from 'outside' and so are not under this Class' control. Unless they are immutable, they are unsafe.


Steve
Mick Peterson
Greenhorn

Joined: Oct 17, 2012
Posts: 5
Thanks for the quick reply!

Paul Clapham wrote:In answer to the questions which you asked: local variables are local to the thread which creates them, so they can't be affected by other threads. In particular since other threads don't have access to them, they can't assign values to them.

So it's the class-level variables you should be concerned about. Like objXMap, for example. And in answer to the question you didn't ask, you don't know that its remove() method is thread-safe. All you know is that it references a Map object, but you don't know whether it's (for example) a ConcurrentMap, which guarantees that remove() is an atomic operation or just any old Map, which doesn't guarantee that.


I am aware that the local variables are thread safe, so that's not my concern. And since it's not part of a public API or anything (I instantiate all the depending classes, including the Map), I know nothing will change the map from the outside.

Rather, my specific concern is whether objX within createResultObj() will always point to the same instance for each thread that extracts if from the map, for the whole duration of the method? I.e:
- thread A calls createResultObj(), and removes objX[a] instance from the map, and begins to execute the rest of the method
- thread B calls createResultObj(), and removes another objX[b] instance from map, before thread A has finished
Question: will thread A always work with the instance objX[a] that it itself got from the map, or can thread B make objX[a] -> objX[b] for thread A if thread B enters the method while thread A is executing it?

I'm having a hard time explaining myself, so if I'm not being clear please let me know.

Oh, and thanks for the welcome!
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

Apparently I'm having the same hard time explaining myself, because I thought the first paragraph I posted specifically answered that question. Let me clarify (perhaps) by pointing out that the only way to change a variable is to assign a value to it.
Mick Peterson
Greenhorn

Joined: Oct 17, 2012
Posts: 5
I think it was the word 'local variable' that threw me off, given that the map is an instance variable, i.e. objX is local, but it is retrieved from the instance variable map.

But just to avoid any misunderstandings; objX is guaranteed to point to the same instance for the duration of the method for each calling thread, even though objX is retrieved from an instance variable within that method?
Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

Mick Peterson wrote:I think it was the word 'local variable' that threw me off, given that the map is an instance variable, i.e. objX is local, but it is retrieved from the instance variable map.

But just to avoid any misunderstandings; objX is guaranteed to point to the same instance for the duration of the method for each calling thread, even though objX is retrieved from an instance variable within that method?


Since objX points to an object that's retrieved from a member variable, other threads could be modifying that spot in the map, other spots in the map, or the stat of the object that objX points to while your method is executing, unless you put appropriate synchronization in place and/or use the tools in java.util.concurrent to prevent it.
Mick Peterson
Greenhorn

Joined: Oct 17, 2012
Posts: 5
Steve Luke wrote:The instance that the variable objX points to will not change during the course of the method call, but there is no guarantee that the data will be in a consistent state.


Thanks, that was my main concern, given that objX is retrieved from an instance variable.

Steve Luke wrote:For example, there would be no guarantee that objXMap.remove(name) will yield an Object, and not null, even if just before this method call you put an ObjX into the map with the correct name. this is not guaranteed because some other thread could be manipulating the map at the same time and remove the object you are trying to work with.

You get two different values from objX. We don't know what objX is, or how safe it is to use. It could be that during the call of this method its data changes such that when you get a and b out of it, those values are inconsistent because the instance of objX has been manipulated.


The code is part of a one-time batch job: I create the map and it's not used anywhere outside of "my" code, so no manipulation of the map will take place while the threads are running (except of course the remove() that each thread calls).
Also, objX, someObj and resultObj are all immutable, so that wouldn't a problem.


Steve Luke wrote:Because you are using the service object, and that service object is being used by different threads, giving it two consistent values as input may produce inconsistent output because the internal data of service may change during the execution of the doSomething() method.


That's a good point to take into consideration.
I guess a synchronized block around the service call is the only solution there.

Steve Luke wrote:Finally, you are adding the results to a List which is also shared among threads. Two threads could be trying to add to the same index, and only one value will get stored. So there is a chance of getting missing or mutilated results.


Another valid point to take into account.
A synchronized block, or using an AtomicInteger perhaps (stored as an instance variable), i.e. resultList.add(atomic.getAndIncrement(), resultObj) ?
Rakesh K. Cherukuri
Ranch Hand

Joined: Jun 01, 2010
Posts: 48

Mick Peterson wrote:But just to avoid any misunderstandings; objX is guaranteed to point to the same instance for the duration of the method for each calling thread, even though objX is retrieved from an instance variable within that method?

I think its not thread safe. Its true that the objX is local and each thread gets its own copy of the variable but its the object thats being assigned should be threadsafe.

Scenario : (assumption is both the threads are getting the same value for the "name" variable)
Thread1 gets reference 'xxxx' from the map and assigns it to its variable copy.
Thread2 gets same reference 'xxxx' from the map and assigns it to its variable copy

Now both the threads are referring to the same object via their own local variable objX copy. That means any changes done to objX instance is not threadsafe.


Warm Regards,
Rakesh
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Mick Peterson wrote:
Steve Luke wrote:Finally, you are adding the results to a List which is also shared among threads. Two threads could be trying to add to the same index, and only one value will get stored. So there is a chance of getting missing or mutilated results.


Another valid point to take into account.
A synchronized block, or using an AtomicInteger perhaps (stored as an instance variable), i.e. resultList.add(atomic.getAndIncrement(), resultObj) ?

I don't think the AtomicInteger approach will work. You will at some point get index out of bounds exceptions because your index is greater than the size of the array (and the add() method does not force the ArrayList to grow). That would mean breaking the step into two (one to ensure capacity, the second to add the value) and once you do that, you need to add synchronized blocks again.

If you are using simple single-step access to the array - for instance you do not iterate over it at all, or do two consecutive calls that need consistency between them - then you might be okay with using Collections#synchronizedList() which wraps the List into a synchronized version that would make single-access steps safe. Anything more than that and you will either need one of the collections in the java.util.concurrent package or external synchronization (or both).
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

Mick Peterson wrote:But just to avoid any misunderstandings; objX is guaranteed to point to the same instance for the duration of the method for each calling thread, even though objX is retrieved from an instance variable within that method?


The only thing that's relevant is the fact that objX is a local variable. It makes no difference whatsoever where it got its value from, or what that value is.

I get the feeling that you're still mixing up variables and objects to a certain extent. It's quite common for programmers to talk as if the variable and the object it contains the reference to are the same thing, but they really aren't. And when you're talking about thread safety, it's important to pay attention to the fact that they are two different things.
Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

Paul Clapham wrote:
The only thing that's relevant is the fact that objX is a local variable. It makes no difference whatsoever where it got its value from, or what that value is.


Eh? Maybe I'm misunderstanding you, but if objX points to an object that can be accessed by other threads (and it appears that it does here), then there are potential thread-safety issues with that object.

Are we talking only about the thread safety of the objX variable istelf, ignoring for the moment whether the object it points to gets mangled?
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

Jeff Verdegan wrote:
Paul Clapham wrote:
The only thing that's relevant is the fact that objX is a local variable. It makes no difference whatsoever where it got its value from, or what that value is.


Eh? Maybe I'm misunderstanding you, but if objX points to an object that can be accessed by other threads (and it appears that it does here), then there are potential thread-safety issues with that object.

Are we talking only about the thread safety of the objX variable istelf, ignoring for the moment whether the object it points to gets mangled?


Yes, exactly. Mick was quite insistent upon that point, saying it was his "main concern". Hopefully the rest of this thread has shown that in fact it's a relatively minor concern in the whole thread-safety tarpit.
Mick Peterson
Greenhorn

Joined: Oct 17, 2012
Posts: 5
Paul Clapham wrote:Yes, exactly. Mick was quite insistent upon that point, saying it was his "main concern". Hopefully the rest of this thread has shown that in fact it's a relatively minor concern in the whole thread-safety tarpit.


You are correct Paul, my concern/question was regarding the variable objX, as the object it references is immutable, and I know that no other code will manipulate map in any way.

Paul Clapham wrote:The only thing that's relevant is the fact that objX is a local variable. It makes no difference whatsoever where it got its value from, or what that value is.

I get the feeling that you're still mixing up variables and objects to a certain extent. It's quite common for programmers to talk as if the variable and the object it contains the reference to are the same thing, but they really aren't. And when you're talking about thread safety, it's important to pay attention to the fact that they are two different things.


Thanks for clarifying that!

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

Mick Peterson wrote:You are correct Paul, my concern/question was regarding the variable objX, as the object it references is immutable, and I know that no other code will manipulate map in any way.


Looks like you are assuming that immutable objects are automatically thread-safe. Have a look at this blog post which I recently read: Does Immutability Really Mean Thread Safety?.
Mike Simmons
Ranch Hand

Joined: Mar 05, 2008
Posts: 3018
    
  10
Having read the article, I am still convinced immutable objects are thread-safe. Is there a part of the article that should convince us otherwise? The author seems incoherent to me.
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18985
    
    8

One thing the article convinced me of was that I don't have a good definition of "immutable". Other people (especially beginners) may have the same problem.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Is this method thread safe?