-1

I found a code snippet online for logging with rollover at midnight to manage the log files so they would be manageable, but running into an error that happens from time to time that I can't find a solution to or rather I might've stumbled upon a solution, but don't know how to implement it. It's using a StreamWriter to write the files.

Not every time, but often it gives an ObjectDisposed exception "Can't open closed file.". The part of the class that is responsible for closing the previous and creating a new file is:

 private void checkRollover()
    {
        // If the date has changed, close the current stream and create a new file for today's date
        if (_currentDate.CompareTo(System.DateTime.Today) != 0)
        {
            try
            {
                _traceWriter.Close();
                _traceWriter.Dispose();
                _traceWriter = new StreamWriter(generateFilename(), true);
            }
            catch (Exception ex)
            {
                MessageBox.Show("This error: " + ex.ToString(), "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }
    }

Is it because of the .dispose at the end as .close does this already or might it be something else? As I said, really stumped by this, any advice on why this might be happening is appreciated. If it's needed I can post the whole class to here.

EDIT Couldn't get the error to happen again, but now it did. I also removed the _tracewriter.Dispose() from there

System.NullReferenceException: Object reference not set to an instance of an object.
  at System.IO.StreamWriter.Flush(Boolean flushStream, Boolean flushEncoder)
 at System.IO.StreamWriter.Dispose(Boolean disposing)
 at System.IO.StreamWriter.Close()
 at FixInterface.RollOverTextWriter.checkRollover()
MihkelT
  • 131
  • 1
  • 8
  • Possible duplicate of [Should I call Close() or Dispose() for stream objects?](https://stackoverflow.com/questions/7524903/should-i-call-close-or-dispose-for-stream-objects) – Krzysztof Bracha Jun 30 '17 at 10:57
  • 2
    Post the complete exception with the stack trace, and make sure we see the code where the exception first occurs. – H H Jun 30 '17 at 11:13
  • The above code isn't thread-safe, so one possibility is a race condition between multiple threads accessing the same _traceWriter instance. – Joe Jul 10 '17 at 08:04
  • Will surrounding the _tracewriter.Close() and the _tracewriter = new ... with lock(_tracewriter) help in your opinion with that? – MihkelT Jul 10 '17 at 08:46

1 Answers1

0

Check out this answer: https://stackoverflow.com/a/7525134/7004050

Do not call Dispose() after Close() because the latter does that implicitly.

Bogdan Anghel
  • 184
  • 3
  • 12
  • 1
    But it's stated there that "_So, reading the code it is clear that that you can call Close() & Dispose() on streams as often as you like and in any order. It won't change the behaviour in any way._" In this case I'm getting an error so that leads me to believe that that isn't the issue. – MihkelT Jun 30 '17 at 11:05
  • Yes, but the `Close()` from the `StreamWriter` calls the garbage collector to physically clear the allocated memory. This could not be done right away by the GC depending on what other tasks it may have. Try the code without `Dispose()`. – Bogdan Anghel Jun 30 '17 at 11:10
  • Dispose/Close [should be safe](https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose) to call multiple times. So while this is a mildly good advice ([CA2202](https://blogs.msdn.microsoft.com/tilovell/2014/02/12/the-worst-code-analysis-rule-thats-recommended-ca2202/)) it is not about the cause of this problem. – H H Jun 30 '17 at 11:16
  • Ok, but can you tell me what .NET version are you using for your code ? – Bogdan Anghel Jun 30 '17 at 11:21
  • Another answer regarding StreamWriter that could help you: https://stackoverflow.com/a/973640/7004050 – Bogdan Anghel Jun 30 '17 at 11:35
  • Would just calling flush be better than closing the stream? – MihkelT Jul 10 '17 at 08:14