Multiple threads writing to 1 file

[WDrago]

All,

I am using lock() to try and sync threads writing to the same file, but my threads are still stepping on one another. Can anyone tell me why this doesn't work and how to fix it?

Here is some sample code. In the real program the writes are very infrequent, happening only when there is an error condition.

// This is Program.cs

using System;
using System.Collections.Generic;
using System.Text;
using System.Threading;

namespace Threadtest
{
    class Program
    {

        static void Main(string[] args)
        {
            WriteToFile wtf1 = new WriteToFile("T1");
            WriteToFile wtf2 = new WriteToFile("T2");

            Thread t1 = new Thread(wtf1.writeJunk);
            Thread t2 = new Thread(wtf2.writeJunk);

            Console.WriteLine("Running...");
            t1.Start();
            t2.Start();
        }
    }
}




// This is WriteToFile.cs

using System;
using System.Collections.Generic;
using System.Text;
using System.IO;
using System.Threading;

namespace Threadtest
{
    class WriteToFile
    {
        private String threadName;
        private System.Object lockThis = new System.Object();

        public WriteToFile(String threadName)
        {
            this.threadName = threadName;
        }

        public void writeJunk()
        {
            while (true)
            {
                try
                {
                    lock (lockThis)
                    {
                        using (StreamWriter outStream = File.AppendText("test.txt"))
                        {
                            outStream.WriteLine("This was written by thread: " + threadName);
                        }
                    }
                }
                catch (Exception e)
                {
                   Console.WriteLine("We should never end up here, but sometimes we do.");
                }

                Thread.Sleep(100);
            }
        }
    }
}

Thanks,
Bill

[Ancient Dragon]

According to this article lockThis needs to be a static object.
static readonly System.Object lockThis = new System.Object();

The above change made your program work for me.

[WDrago]

Thanks for the reply, Dragon. Your suggestion makes things worse for me. I am using VS2010 and my target framework is .NET 4.0.

According to Microsoft lockThis can be a private object or a private static variable.

At any rate, comment out the line Thread.Sleep(100); and you'll see what I'm experiencing here.

I have a feeling that C# is indeed locking the critical section of code, but Windows is not closing the file in a timely fashion. I might have to check to see if the file is available before trying to write to it.

-Bill

[Ancient Dragon]

That Microsoft article is incorrect. The program causes an exception if LockThis is anything other than static.

There is not guarentee that threads will execute in consecutive order, as you can see from the output in that text file. You should not expect to see alternating lines of thread t1 and t2 in the file unless you tell the thread to release cpu time back to the MS-Windows task scheduler. I assume this will probably not be that big a problem for you in a complete program where each thread also has other tasks to do.

Is your intent to creat a program that produces a log file? If it is, there may be alternate ways to accomplish that.

[caKus]

Must'n you close the file after writing ?

[Ancient Dragon]

No. The file is closed when leaving the lock block of code.

[WDrago]

Must'n you close the file after writing ?

That's what using is for. See: Your Friend the C# Using Statement

[WDrago]

There is not guarentee that threads will execute in consecutive order, as you can see from the output in that text file. You should not expect to see alternating lines of thread t1 and t2 in the file unless you tell the thread to release cpu time back to the MS-Windows task scheduler.

I do not expect to see alternating T1/T2. The real program has 4 threads running. If more than one or them tries to write to the file at the same time (extrememly rare, but I have to allow for it) I get a System.IO.IOException: "The process cannot access the file...because it is being used by another process." The demo program I posted here illustrates the problem nicely.

Is your intent to creat a program that produces a log file?

Not exactly, the program logs all data in a SQL Server database. This is just an error log file. Errors are extrememly rare, errors in more than one thread at a time are even rarer, but as I said, I have to allow for it.

So, there must be a standard way of handling the possibility of multiple threads trying to write to a file at the same time. I don't want to reinvent the wheel, I'm just asking what the normal way of doing this is.

BTW, if anyone actually runs the sample I provided you might think it's working great. Just let it run form a while and sooner or later it'll start to misbehave.

Thanks,
-Bill

[Ancient Dragon]

The program you posted doesn't give me any exceptions (vs 2012), works as expected. As long as lockThis is declared readonly there should be no problem.

[WDrago]

The program you posted doesn't give me any exceptions

Hmmm. Okay, maybe I have some other kind of problem here. Thanks for your help...

-Bill

[WDrago]

According to this article lockThis needs to be a static object.
static readonly System.Object lockThis = new System.Object();

The above change made your program work for me.

And now it works for me too. I don't know what I was doing wrong yesterday, but today it's working just fine.

I guess part of the problem is that I don't understand the purpose of lockThis. It seems so arbitrary and has abosultely nothing to do with the code block that I want locked. Can anyone explain?

Thanks,
Bill

[Ketsuekiame]

Imagine you're at a park, that has one entrance in and out, which is protected by a lockable gate. This is a very exclusive park and so only one person is allowed in at a time. When a user enters the park they are given the key, they open the gate, walk in. As the gate closes, it locks itself automatically.

Now some others want to get in the park, but they have to wait for the first person to exit. When this person leaves the park, they hand the key to the next person in line so they may enter.

So you have the users (the threads), a park (the code), a gate and lock (the lock statement) and a key (the object lockThis)

The reason it needs to be static, is so that there is only a SINGLE key, no matter how many instances of the parent object (writeToFile) you make, otherwise, whenever you made a new instance of writeToFile you'd be generating a new key and the lock would be useless.

Hope this has helped explain it and not made matters worse for you ;)

[WDrago]

Ketsuekiame: Thanks for the reply, I already figured it out. Here's how I explained it to myself: lockThis is simply a token. Whoever has the token can run, the other threads have to wait until the token becomes available. And, of course, there can only be one token.

My thanks to all who helped me not only solve the problem, but understand the solution as well.

-Bill