Fix filetarget: Thread was being aborted (#2)

[304NotModified]

Need for Thread.ResetAbort();

Fixes #1385

See http://stackoverflow.com/questions/1856286/threadabortexception

Thanks @jonreis!

[jonreis]

Thank you 304Modified for attempting to fix this issue, but it is not quite there.

            in                lock (SyncRoot)
                              this.fileAppenderCache.InvalidateAppendersForInvalidFiles();

The call on lock(SyncRoot) will also cause a ThreadAbortException which is currently not caught

I think using Thread.Abort() is just bad practice anyway and is not needed in this context (http://stackoverflow.com/questions/2251964/c-sharp-thread-termination-and-thread-abort).

Instead, what about using a ManualResetEvent, or the lighter weight ManualResetEventSlim on .NET 4.5? Something like:

private ManualResetEvent appenderManualResetEvent;

                    if ((EnableArchiveFileCompression) && (this.appenderInvalidatorThread == null))
                    {
                        // EnableArchiveFileCompression creates a new file for the archive, instead of just moving the log file.
                        // The log file is deleted instead of moved. This process may be holding a lock to that file which will
                        // avoid the file from being deleted. Therefore we must periodically close appenders for files that 
                        // were archived so that the file can be deleted.
                        this.appenderManualResetEvent = new ManualResetEvent(false);
                        this.appenderInvalidatorThread = new Thread(new ThreadStart(() =>
                        {
                          try
                          {
                            while (true)
                            {
                              if (appenderManualResetEvent.WaitOne(200))
                                break;

                              lock (SyncRoot)
                                this.fileAppenderCache.InvalidateAppendersForInvalidFiles();
                            }
                          }
                          catch (Exception ex)
                          {
                            InternalLogger.Info("Error in InvalidatorThread loop {0}", ex);
                          }
                        }));

                        this.appenderInvalidatorThread.IsBackground = true;
                        this.appenderInvalidatorThread.Name = "NLog AppenderInvalidatorThread";
                        this.appenderInvalidatorThread.Start();
                    }

Stop would look like this:

  private void StopAppenderInvalidatorThread()
  {
      if (this.appenderManualResetEvent == null)
         return;

    try
    {
        this.appenderManualResetEvent.Set();
    }
    catch (Exception ex)
    {
      InternalLogger.Info("StopAppenderInvalidatorThread error {0}", ex);
    }
  }

[304NotModified]

Thanks for the info. I think it would be nice to fix this without thread.

Is it possible to send a PR for this?

[jonreis]

If you must use Thread.Abort() the code should look like:

                            while (true)
                            {
                                try
                                {
                                    Thread.Sleep(200);

                                  lock (SyncRoot)
                                    this.fileAppenderCache.InvalidateAppendersForInvalidFiles();
                                }
                                catch (ThreadAbortException ex)
                                {
                                    //ThreadAbortException will be automatically re-thrown at the end of the try/catch/finally if ResetAbort isn't called.
                                    Thread.ResetAbort();
                                    InternalLogger.Trace(ex, "ThreadAbortException in Thread.Sleep");
                                     break; // this is the addition. Need to exit the loop
                                }
                                catch (Exception ex)
                                {
                                    InternalLogger.Warn(ex, "Exception in Thread.Sleep, most of the time not an issue.");
                                } 
                            }

[jonreis]

I logged 1415, with a fix, but like I said in the PR, I don't know why there is a dedicated thread being spun up to do this. Could it not just be a simple timer?