[Thread-Safety] Lock Object

[Cool Guy]

I have an application log file class which is used throughout the app,
potentially by multiple threads. It's a singleton.

Here're the relevant parts:

sealed class LogFile
{
#region Singleton functionality
public static readonly LogFile Instance = new LogFile();

static LogFile() {
}
#endregion

StreamWriter streamWriter;
readonly object streamWriterLock;

LogFile() {
#if TRACE
streamWriterLock = new Object();
FileStream fileStream = File.Open(...);
lock (streamWriterLock) { // lock for write memory barrier
// (not sure whether this is needed)
streamWriter = new StreamWriter(fileStream, Encoding.ASCII);
}
#endif
}

public void AddEntry(string text) {
#if TRACE
lock (streamWriterLock) {
streamWriter.WriteLine(text);
streamWriter.Flush();
}
#endif
}
}

Now, obviously I lock in AddEntry for atomic writing to the file.

But my question is: should /streamWriterLock/ be volatile?

There could be a situation where, for example, thread A and thread B are
both running, and thread A accesses Instance thus creating the instance.
Then thread B accesses it. Would thread B definitely see the right value
for /streamWriterLock/ in this case?

 

[Lloyd Dupont]

I would "assume" that class loading & initialization is a thread safe
operation.
This assumption is enforced by the "Threading Desing Guidelines" from .NET
documentation (bullet n2):

a.. Avoid providing static methods that alter static state. In common server
scenarios, static state is shared across requests, which means multiple
threads can execute that code at the same time. This opens up the
possibility for threading bugs. Consider using a design pattern that
encapsulates data into instances that are not shared across requests.

a.. Static state must be thread safe.

a.. Instance state does not need to be thread safe. By default, class
libraries should not be thread safe. Adding locks to create thread-safe code
decreases performance, increases lock contention, and creates the
possibility for deadlock bugs to occur. In common application models, only
one thread at a time executes user code, which minimizes the need for thread
safety. For this reason, the .NET Framework class libraries are not thread
safe by default. In cases where you want to provide a thread-safe version,
provide a static Synchronized method that returns a thread-safe instance of
a type. For an example, see the System.Collections.ArrayList.Synchronized
Method and the System.Collections.ArrayList.IsSynchronized Method.

a.. Design your library with consideration for the stress of running in a
server scenario. Avoid taking locks whenever possible.

a.. Be aware of method calls in locked sections. Deadlocks can result when a
static method in class A calls static methods in class B and vice versa. If
A and B both synchronize their static methods, this will cause a deadlock.
You might discover this deadlock only under heavy threading stress.

a.. Performance issues can result when a static method in class A calls a
static method in class A. If these methods are not factored correctly,
performance will suffer because there will be a large amount of redundant
synchronization. Excessive use of fine-grained synchronization might
negatively impact performance. In addition, it might have a significant
negative impact on scalability.

a.. Be aware of issues with the lock statement (SyncLock in Visual Basic).
It is tempting to use the lock statement to solve all threading problems.
However, the System.Threading.Interlocked Class is superior for updates that
must be atomic. It executes a single lock prefix if there is no contention.
In a code review, you should watch out for instances like the one shown in
the following example.