I was creating a small logger. For that purpose I have a class (LogFile
) which publishes the Log(ILogCsvLine csvLine)
method. This method adds the line to log to a queue (linesToLog
) and sets a trigger, that has been registered to the ThreadPool with Logging(..)
as method that should be executed on a different thread whenever there is a trigger and pending processor time. The Logging(..)
method is writing the lines to log to a given file.
Now I ran into the problem, that the Dispose()
method has been called, while the queue wasn't empty, resulting in a call to the Logging(..)
method, while trigger
or fileAccessLock
were already disposed. As a solution I've build some checks around those EventWaitHandles
and was wondering whether there is a better readable and more elegant way of doing this.
internal sealed class LogFile : ILogFile
{
private readonly EventWaitHandle fileAccessLock = new AutoResetEvent(true);
private readonly IFilesLoader filesLoader;
private readonly Queue<ILogCsvLine> linesToLog = new Queue<ILogCsvLine>();
private readonly IFile logFile;
private readonly object myLock = new object();
private RegisteredWaitHandle registeredWait;
private readonly EventWaitHandle trigger = new AutoResetEvent(false);
private IDirectory directory = null;
private bool disposeFileAccessLock = false;
private bool disposeTrigger = false;
private bool isDisposed = false;
private bool isDisposing = false;
private bool isLogging = false;
private bool isSettingTrigger = false;
public event EventHandler<FilesException> LoggingFailed;
public LogFile(IFile logFile, IFilesLoader filesLoader)
{
this.filesLoader = filesLoader;
this.logFile = logFile;
Setup();
}
private void Setup()
{
directory = logFile.ParentDirectory;
EnforceFileExists();
registeredWait = ThreadPool.RegisterWaitForSingleObject(trigger, Logging, null, -1, false);
}
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
~LogFile()
{
Dispose(false);
}
private void Dispose(bool disposing)
{
if (!isDisposed)
{
try
{
lock (myLock)
isDisposing = true;
registeredWait?.Unregister(trigger);
if (disposing)
{
if (isSettingTrigger)
disposeTrigger = true;
else
trigger?.Dispose();
if (isLogging)
disposeFileAccessLock = true;
else
fileAccessLock?.Dispose();
}
}
finally
{
isDisposed = true;
lock (myLock)
isDisposing = false;
}
}
}
public IFile File => logFile;
public void Log(ILogCsvLine csvLine)
{
lock (myLock)
{
if (isDisposing || isDisposed)
return;
linesToLog.Enqueue(csvLine);
isSettingTrigger = true;
}
trigger.Set();
lock (myLock)
{
isSettingTrigger = false;
if (disposeTrigger)
trigger?.Dispose();
}
}
private void Logging(object data, bool timedOut)
{
ILogCsvLine line = null;
lock (myLock)
{
if (linesToLog.Count == 0)
return;
if (isDisposing || isDisposed)
return;
line = linesToLog.Dequeue();
isLogging = true;
}
fileAccessLock.WaitOne();
FilesException occurredException = null;
IStreamWriter sw = null;
try
{
EnforceFileExists();
sw = logFile.AppendText();
do
{
sw.WriteLine(line.ToCsvString());
lock (myLock)
{
if (linesToLog.Count > 0)
line = linesToLog.Dequeue();
else
line = null;
}
} while (line != null);
}
catch (Exception e)
{
if (e is ThreadAbortException)
throw;
string message = string.Format("Error writing to {0}. {1}", logFile.Path, e.Message);
occurredException = new FilesException(message, e);
}
finally
{
if (sw != null)
{
sw.Flush();
sw.Close();
sw = null;
}
}
fileSizeManager?.Check();
fileAccessLock.Set();
lock (myLock)
{
if (disposeFileAccessLock)
fileAccessLock?.Dispose();
isLogging = false;
}
if (occurredException != null)
LoggingFailed?.Invoke(this, occurredException);
}
private void EnforceFileExists()
{
if (!directory.Exists)
directory.Create();
if (!logFile.Exists)
{
var fileAccess = filesLoader.GetFileAccess();
fileAccess.Create(logFile.Path, FileSystemRights.Read | FileSystemRights.Write);
}
}
}