How to prevent a method of a class to be called before the class is ready ?

Question

Still exploring C# ... so bare with me please.

Here is my custom "logger" class, which helps me create logs file in my project.

namespace MyProject
{
class Logger
{
    private FileInfo fFile;
    private DirectoryInfo dDir;

    /// <summary>Add a new entry to the log file.</summary>
    /// <param name="sData">The line to add.</param>
    public void Add(string sData)
    {
        DateTime CurrTime = DateTime.Now;

        if (fFile.Length > 1048576)
        {
            fFile.MoveTo(Path.Combine(dDir.FullName, CurrTime.ToShortDateString() + fFile.Name));
            fFile = new FileInfo(Path.Combine(dDir.FullName,fFile.Name));
            using (StreamWriter sw = fFile.CreateText())
            {
                sw.WriteLine("{0:u}|{1}", CurrTime, sData);
            }
        }
        else
        {
            using (StreamWriter sw = fFile.AppendText())
            {
                sw.WriteLine("{0:u}|{1}", CurrTime, sData);
            }
        }
    }

    /// <summary>Logger instance</summary>
    /// <param name="sFile">Full name of the file to use as logs. Ex : "MyLogs.txt"</param>
    public Logger(string sFile)
    {
        dDir = new DirectoryInfo(Path.Combine(MyProject.AppPath, "logs"));
        if (!dDir.Exists)
        {
            dDir.Create();
        }

        fFile = new FileInfo(Path.Combine(dDir.FullName,sFile));

        if (!fFile.Exists)
        {
            using (StreamWriter sw = fFile.CreateText())
            {
                sw.WriteLine("{0:u}|Logger Started", DateTime.Now);
            }
        }
        else
        {
            Add("Logger Started");
        }           
    }
}
}

The problem I have with this code is, apparently, sometimes, Logger.Add is called before the new instance of that logger had time to create the file. So my program crashes saying "file not found", although, the file ends up being created in the end, and if I restart my program using the same filename for the logs, everything works fine (because the file exists now ...)

Instead of just making sure logger.add is not called before the file is created, is there a way to "lock" the class ?

I've tried the lock method but it didn't work ... Lock(this) didn't do anything, and I can't use it on the method itself.

Answer 1

Personally I wouldn't go this route, with locking the class. I would take the code out which creates the log file and make it its own method. Then when the add method is called, add in some logic to check to see if the log file exists (this logic should also be its own method, in my opinion). If the file exists, continue on with logging, if not then create the log file (using the method you extracted above), then once that method returns successfully, go ahead and write your log.

Answer 2

In the method, check if the class "is ready" (whatever that may be) and if it's not either wait, return an error or whatever you need to do.

Answer 3

You might attempt using the FileSystemWatcher class and handle events to make the class ready. That way the event could be added as a delegate and would only be called once the file in question was created. Thread.Sleep might be processor intensive and cause other processes to lock up while this process is expecting to complete.

Answer 4

In your constructor you are setting the field fFile and dDir, instead you should have a field that is a streamwriter, and use THAT in the Add method.

Answer 5

The poblem is that the IO operation has been cached. This theoretically shouldn't be a problem but practically it is.

You can call sw.Flush() in your constructor. That will force the file from the cache to the disk and therefore to create the file.

if (!fFile.Exists) 
{ 
    using (StreamWriter sw = fFile.CreateText()) 
    { 
        sw.WriteLine("{0:u}|Logger Started", DateTime.Now); 
        sw.Flush();
    } 
}

Answer 6

You want to prevent your add method from running before the file is created - you can do this using an EventWaitHandle or perhaps some other thread control mechanism (perhaps using the monitor class).

And also restrict usage of the file to one thread at a time using Lock, you don't want two threads running Add at the same time - and both trying to move the file at once.

EventWaitHandle _handle = new EventWaitHandle (false, EventResetMode.ManualReset);
Object _fileLock = new Object();


 public Logger(string sFile)
{

// Do as you do here

_handle.Set();
}


public void Add(string sData)
{
    _handle.WaitOne ();    // Blocks the thread untill Set() is called on _handle


    Lock(_fileLock){    // Only one thread may enter at a time
       // Do as you do already
    }

}

On second thought, using Lock(_fileLock){ around your ctor code should do the same job as the WaitHandle.

Answer 7

EDIT - I wrote this on a Linux machine when I didn't have a C# compiler around. Having tested the code on a Windows machine I've found the problem observed here is in fact much simpler than my speculation. See my other answer.

Are you using multiple threads here? If you are, you're going to need to take a different approach. For example, you will suffer from race conditions if two threads attempt to call Add at the same time and one or both attempt to move the file.

Can any other process move or lock your log-file while your program is running, and if they do, does your program need to not crash? If that is the case, you're going to need to change a lot, since all of your file I/O operations might fail.

If neither of these cases apply, then it looks like your problem is that the file is stored on some file system that exhibits some amount of latency between you creating a file and you being able to observe that file. I did not think this was the case with NTFS or FAT32. Are you storing the log file on a network share? Without knowing more, I can't think of much better to do than employing rockinthesixstring's suggestion of testing if the file exists and if not using a short Sleep() call until it does (or until you've waited long enough that it's clear something has gone wrong - maybe somebody deleted the file from under you).

Note that if the problem is related to the file system, no amount of locking in the class is going to help, nor will calling Flush because the "using" statement ensures that the StreamWriter has been disposed, which will close the file in addition to having much the same effect as Flush, so a Flush is redundant.

All that said, why are you closing the file and opening it again all the time? Is it to make sure nothing goes missing if your program crashes? If so, you are probably better off following jmoreno's advice to keep a TextWriter open, but to call Flush on it after you write each line.

Answer 8

The class is supposed to be ready when its constructed as fas as the compilers job is concerned. Beyond that any logic will have to be programmed.

    fFile.MoveTo(Path.Combine(dDir.FullName, CurrTime.ToShortDateString() + fFile.Name));
    fFile = new FileInfo(Path.Combine(dDir.FullName,fFile.Name));

This looks like the race condition and needs to be serialized. We had one off issues even after using locks (perl/log4perl) and ended up preferring separate log files for each of the processes. Unless there are multiple Logger objects alive (even across processes since we are dealing with files) it should not be an issue.

Wonders, isn't there a log4 equivalent in C#.

Answer 9

The exception is actually caused not because the file doesn't exist, but because the FileInfo instance is stale! You created the FileInfo when the file didn't exist, and it takes a snapshot of the state of the file at this time. When I test it the exception is getting thrown when you invoke fFile.Length in the Add method. If I add in a call to fFile.Refresh() I find it works:

...
DateTime currTime = DateTime.Now;
fFile.Refresh();
if (fFile.Length > 1048576)
...

See here:

http://msdn.microsoft.com/en-us/library/system.io.filesysteminfo.refresh.aspx

"Calls must be made to Refresh before attempting to get the attribute information, or the information will be outdated."