views:

89

answers:

4

I have this question.

public class Foo : object
{
    public override bool Equals(obj a, objb)
    {
       return ((Foo)a).Bar.GetHashCode() == ((Foo)b).Bar.GetHashCode();
}

}

Suppose I want to make Foo threadsafe. Do I need to synchronize calls to GetHashCode()?

A: 

Even if you were going to add synchronization, what would you synchronize against?

I would say as a general rule GetHashCode should always be threadsafe, but the docs don't state a requirement for thread safety. A consumer of Bar can't really guarantee it's usage is threadsafe unless Bar provides thread safety internally.

Let's say Bar uses two internal fields to calculate the hash code. If another thread is in the process of updating the fields and only one has changed, you'll be able to get a GetHashCode() result after one field was updated but before the second was updated, unless Bar internally provides synchronization.

Sam
+1  A: 

From the documentation for System.Object:

Public static members of this type are thread safe. Instance members are not guaranteed to be thread-safe.

Thus, if you have not overrode Object.GetHashCode you have no guarantee of thread safety. If you have overrode it depends on your implementation. It also depends on what you mean by "thread safe."

Jason
A: 

It really depends on what Bar is. If bar is a "readonly" string field on Foo, then it is thread-safe. If bar is a read-write field holding an arbitrary value type with its own definition of GetHashCode() then there may be nothing thread-safe about your function at all.

It also depends on your definition of thread safety. Do you define thread safety to mean that the returned value is consistent for all time? If so, then there is no way for that to be guaranteed unless the value of Bar's hash code cannot change. If not, then you have to acknowledge that the return value of this method could be incorrect given the current states of a and b before it has even returned.

If Bar can change, then this is perhaps an intractable problem. If you try to lock a and b (or lock private mutexes in a and b), which order do you do it in? What if the caller reverses the order of the parameters? That will create a very straightforward deadlock. Google the concept of a "lock hierarchy" to see the dangers therein. If Bar can change, the only way to make this operation thread-safe is to lock write access to all Bar's that could be changed with one global lock prior to doing the comparison.

David Gladfelter
A: 

I can't see a compelling reason why you'd need to make GetHashCode thread-safe, and I could see it leading to deadlocks or at least lock convoys if you aren't careful; GetHashCode is called from a lot of places you might not expect.

What's more logical is that if you have multiple threads sharing a single Foo, those threads will need to synchronize their access to the Foo, so you simply don't have multiple threads calling GetHashCode at once (or any other method of Foo).

Correctly designing thread-safe classes can be a difficult exercise; unless you have a very good reason to require thread-safe instance members, I would choose not to do it. The vast majority of the .NET Framework itself does not have thread-safe instance members.

Finally, there really isn't such a thing as a class being universally thread-safe. You can synchronize every single instance member, but a consumer of your class can still mess it up if they don't understand the semantics. Making GetHashCode thread-safe implies that you are trying to account for every conceivable threading scenario, but even most thread-safe classes are only thread-safe for certain operations, and GetHashCode is not normally one of them.

I'll refer you to Eric Lippert's recent post on thread-safety; think about the content of this when using a phrase such as "Make Foo thread-safe."

Aaronaught