Why the lock in this code is not working?

Question

With this code for a very basic logger:

lock (string.Concat("LogWritter_", this.FileName))
{
    using (var fileStream = File.Open(this.FileName, FileMode.Append, FileAccess.Write, FileShare.Read))
    {
        using (var w = new StreamWriter(fileStream))
        {
            w.Write(message);
        }
    }
}

when I try it from a few threads simultaneously I quickly get the error:

The process can't access the file because its being used by another file.

Why the lock is not preventing the threads to access the file at the same time?

It doesn't matter if the threads call the same instance or different instances to the same file. Also I thought It could be because of some deferral when writing files in Windows, but on Linux happens the same thing.

Answer 1

You are locking on a temporary string. You have to introduce a static object to lock on.

Answer 2

Create a Dictionary<string,object> and store your lock objects there with the filepath as key.

A while back, I approached this same question:

Locking by string. Is this safe/sane?

Answer 3

The C# lock statement places a lock on the object, not the uniqueness of the string. So, because you are dynamically concatenated two strings, you are essentially creating a new object every single time, so every single lock is unique. Even if you access the same file every time, "A" + "B" results in some new immutable string; "A" + "B" again results in yet another new object.

Answer 4

You are only locking a dynamically created string ("LogWritter_" + this.FileName)! Each thread will create another one. Create a common lock-object instead

public static readonly object fileLock = new object();

...

lock (fileLock) {
    ...
}

---

If you want to create different locks for different files, you will have to store them in a collection that will be used by all the threads.

If you are working with .NET Framework 4.0, you can use a ConcurrentDictionary<TKey, TValue>. Otherwise you will have to lock the access to a normal Dictionary<TKey, TValue>

public static readonly ConcurrentDictionary<string,object> fileLocks =
    new ConcurrentDictionary<string,object>();

...

object lockObject = fileLocks.GetOrAdd(filename, k => new object());
lock (lockObject) {
    ...
}

---

UPDATE

If you want to compare the references of two strings, you have to use

Object.ReferenceEquals(s1, s2)

Where

string s1 = "Hello";
string s2 = "Hello";
Console.WriteLine(Object.ReferenceEquals(s1, s2)); // ===> true

string s3 = s1 + " World!";
string s4 = s2 + " World!";
Console.WriteLine(s3 == s4); // ===> true
Console.WriteLine(Object.ReferenceEquals(s3, s4)); // ===> false

Strings created at compile-time are interned, i.e. a single string constant will be created for equal strings. Strings created at runtime, however, will be created as individual and distinct objects!

The hash code of strings is calculated from the characters of the string, not from their reference.

Answer 5

Try out this code. When the first thread comes in and calculates the value of string.Concat("LogWritter_", this.FileName) it puts a lock on this string. The second thread will also calculates the same string value but the strings are going to be different. If you will compare strings using ==,Equals() or GetHashCode() you will see both strings are same because == and Equals() are overloaded for string class. but if you will check the ReferenceEquals() then you it returns false. It means both the string have different references. And that is why first thread locks on one string object and second thread locks on second string object and you get the error.

class Program
{
    public static void Main(string[] args)
    {
        string locker = "str", temp = "temp";
        string locker1 = locker + temp;
        string locker2 = locker + temp;

        Console.WriteLine("HashCode{0} {1}", locker1.GetHashCode(), locker2.GetHashCode());
        Console.WriteLine("Equals {0}", locker1.Equals(locker2));
        Console.WriteLine("== {0}", locker1 == locker2);
        Console.WriteLine("ReferenceEquals {0}", ReferenceEquals(locker1, locker2));
        app.Program p = new Program();
        Action<string> threadCall = p.Run;
        threadCall.BeginInvoke(locker1, null, null);
        threadCall.BeginInvoke(locker2, null, null);
        Console.Read();
    }

    public void Run(string str)
    {
        lock (str)
        {
            Console.WriteLine("im in");
            Thread.Sleep(4000);
            Console.WriteLine("print from thread id {0}", Thread.CurrentThread.ManagedThreadId);
        }
    }


}