.NET thread safety

Question

The ApplicationException constructor is an instance member so it is not guaranteed to be thread-safe. How do you know if the following code is completely unnecessary? When I say know, I mean is there something in the documentation that says this is unnecessary or have you seen the .NET source code and so you know it is unnecessary?

// thread-safe
internal static class TsApplicationException
{
    private readonly static object myLock = new object();

    internal static void Throw(string msg, Exception e)
    {
        lock (myLock)
        {
            throw new ApplicationException(msg, e);
        } 
        // the implementation of lock() ensures that the unlock will happen
        // even if there is an exception thrown
    }
}

Answer 1

This is unnecessary because properties within the ApplicationException object aren't going to be changed, and I believe all properies in exception objects are read-only.

Therefore thread-safety is irrelevant.

Answer 2

Firstly, instance members can be threadsafe. It's just common to make sure ones static members are threadsafe but not provide the same guarantees for instance members for the reasons we discussed previously.

Secondly, the constructor is not being called in a context where more than one thread can access the same instance. The only reference to the newly constructed ApplicationException is, at that point, local to the calling method, and hence only visible to one thread. If two threads both hit the Throw method at the same time, they will have two separate instances created.

Therefore while it is not thread-safe per se, it is not used in a context accessible to multiple threads, and hence there is no need to lock on it.

A more important case is:

void MyFunctionOfThreadSafety(string someStr, int someInt, bool someBool)
{
  if(someBool)// №1
  {
    var dict = new Dictionary<string, int>();
    dict[someStr] = someInt; // №2
    int test = 0;
    if(!dict.TryGetValue(someStr, out test))// №3
      throw new Exception("really unexpected");
    dict[someStr] = test + someInt; // №4
  }
}

In this code, the lines commented with №1 through №4 are places where if the objects in question where accessible by more than one thread, then thread-safety issues could cause problems (indeed, all but №1 offer more than one point at which threads could switch and things start being weird).

This code though is entirely thread-safe. Although it is doing several potentially unsafe things with objects, not a single one of those objects is changeable by another thread.

someInt and someBool are value-type parameters that were not passed byref or out. They therefore only exist in the context of that method, (and methods it calls if it passes them on byref or out).

someStr is a reference type passed as a parameter, which means it could potentially be also stored somewhere that another thread can get at it. However, because it is immutable, there is no danger of another thread writing to it (since no thread can write to it at all). Reads do not have to be protected from reads, just from writes (though when you do have reads and writes you may have to lock both the read and the write). Since there can only be read operations on someStr it's safe.

dict and test are created in this method, so they are safe. If dict were returned from the method then as a mutable reference type it could, if stored somewhere visible to multiple threads, become a thread-safety concern. But (a) it has not been and (b) it won't be a concern until after this has happened; here it is thread-safe no matter what.

Answer 3

The only time constructors are not thread-safe is when they alter static data in their class. This is extremely rare, so the locking is unnecessary. Indeed, you are only locking your usage of the constructor, so any other thread not running in TsApplicationException can throw an ApplicationException simultaneously anyway.