views:

104

answers:

2

How do I make code like this thread safe?

public static class Converter
{
    public static string ConvertNameToNickname(string name)
    {
        if (name.Equals("John"))
        {
            return "Johnathon";
        }

        return name;
    }
}

Or is it already thread safe since "name" is a local variable? I just want to make sure if ConvertNameToNickname was called by two different threads the name it was evaluating wasn't being stepped on by other threads.

<--edit-->

Ok some of these answers have been pretty helpful but I still haven't ferreted out the answer I was looking for so let me modify it a bit and ask the same question. How would I make this code thread safe given a mutable-type parameter? Or is it even possible? If I throw a lock{} around the entire method body (shown in the example 2) is it still possible for the instance variable "name" to be modified before we enter the lock statement block?

public static class Converter
{
    public static string ConvertNameToNickname(StringBuilder name)
    {
        if (name.ToString().Equals("John"))
        {
            return "Johnathon";
        }

        return name;
    }
}

Example 2:

private static readonly object _lockObject = new object();

public static class Converter
{
    public static string ConvertNameToNickname(StringBuilder name)
    {
        lock(_lockObject)
        {
            if (name.ToString().Equals("John"))
            {
                return "Johnathon";
            }

            return name;
        }
    }
}
+1  A: 

Yes, it's already thread safe since "name" is a local variable assigned to an immutable object.

Local parameters are passed by value, so a copy of the value/reference is always made, as opposed to a ref parameter in which case you might have concurrency issues if it was a reference to a field.

Mark Cidade
That doesn't make the method magically become thread-safe. What if you pass the same instance to the method in two threads at the same time?
dtb
This is thread-safe, but not because name is local. That's a completely different issue.
Reed Copsey
In this particular case, it would still be fine since strings are immutable. In general, local parameters of mutable instances of reference types wouldn't be thread-safe.
Mark Cidade
+5  A: 

Or is it already thread safe since "name" is a local variable?

Close. It is threadsafe, but not because of the local variables. However, since string in .NET is immutable, this is thread safe. The key to being thread safe without synchronization is always working with immutable types. With mutable types, even methods like .Equals may not be thread safe.

Were string a mutable class, this would not necessarily be thread safe.


Edit:

Ok some of these answers have been pretty helpful but I still haven't ferreted out the answer I was looking for so let me modify it a bit and ask the same question. How would I make this code thread safe given a mutable-type parameter? Or is it even possible?

Yes, it is potentially possible. However, it requires synchronization of some form to be correct. Using a lock is one option.

If I throw a lock{} around the entire method body (shown in the example 2) is it still possible for the instance variable "name" to be modified before we enter the lock statement block?

Unfortunately, it is. The lock prevents this code from being called simulatenously from two different threads. In essence, you're making this specific use of "name" thread safe. However, if "name" is used elsewhere in your program, it's still possible that it can be modified by some other function while being used within your lock. This could lead to a subtle race condition.

Thread-safety is tough to get right. The best option for thread-safe usage of mutable types is usually to provide your own, thread safe types - where the type itself handles all of its required synchronization internally, and the public API is completely thread safe. This is the only way to guarantee that nobody else will "mess with" your data, even if you're locking around the usage.

This is where immutable types come into play - they eliminate all of these worries, since their state can't be changed once created. Anybody can use them, and construct more, without risk to the type itself.

Reed Copsey
I think a key point to make here is that name is not a local variable, it is a parameter whose type is a reference type. If the parameter were of a reference type that was mutable the code would not be thread safe. If the parameter were of any value type the code would be thread safe, in that no other thread could change the value. As Reed pointed out, string is immutable, its value once set cannot be changed; therefore as constructed the code is thread safe.
Steve Ellinger
I see what you are saying in your post but I didn't initially provide a good enough example to get the type of information I was looking for. Can you look at my edit and see if that changes the information in your response at all?
omatase
@omatase: Does that help? I just edited to add more info based on your questions...
Reed Copsey
So the only way in which option two is not thread safe is if there is another reference to the name variable somewhere else in the code? That makes sense to me, but is option 2 thread safe assuming you always create the StringBuilder instance in the thread in which you are making the method call to ConvertNameToNickname? Or is it possible that the reference for name can be replaced by a caller passing a new reference to name in a separate thread before the lock statement?
omatase
@omatase: Well, since "name" is a reference type, and other references pointing to that same instance have the ability to mutate it. If another reference is being used in a different thread while you're in that lock, that other thread (as long as it's not locked around the same lock object) can mutate it "under you", and cause problems. If the only usage of that variable, in any code, is inside locks locking around the same object, then it's safe.
Reed Copsey