C#: Cannot write to a closed TextWriter

Question

why when the program hit //HERE. it crashed with Unhandled Exception: System.ObjectDisposedException: Cannot write to a closed TextWriter.? Print() method is called before LogClose(). What I want to achieve is if it hits HERE then print out the message and program exit

foreach (string sf in ff)
{

    if (File.Exists(sf))
    {
        File.Move(sf, copy_dir + "\\" + sf);
    }
    else
    {
        //HERE
        l.ExitAndOut("Error: " + sf + " not found");
    }
}



class logger {
StreamWriter sw = new StreamWriter("SpireCli.log");

public void Print(string log)
{
    DateTime ts = DateTime.UtcNow;
    sw.WriteLine(ts + " " + log);
}

public void ExitAndOut(string log)
{
    string Program_Exit = "Program Exit";

    Console.WriteLine(log);
    Console.WriteLine(Program_Exit);
    Print(log);
    Print("Exit");
    LogClose();
    Environment.Exit(1);
}

public void LogClose()
{
    sw.Close();
}
}

the entire code needs to be rewritten. this is very poor design and you are likely to run into other issues writing it like this...instead of `sw.Close`, try `using` keyword which will automatically take care of things like that. — Timmerz, Apr 24 '13 at 20:29
@zimdanen nope, loop will hard terminate with `Environment.Exit(1);`. — Jesse C. Slicer, Apr 24 '13 at 20:30
@Timmerz - interesting advice, but `using` can't span multiple methods :) — Alexei Levenkov, Apr 24 '13 at 20:31
@Timmerz: can you give me more info. I am intrigue by your suggestion. — John Ryann, Apr 24 '13 at 20:31
@AlexeiLevenkov note in the context I said the "entire code needs to be rewritten" — Timmerz, Apr 24 '13 at 20:31
Print() method is called before LogClose(). why the complain? — John Ryann, Apr 24 '13 at 20:34
Missing many important details here. Where is declared, initialized and opened the variable sw? Did the crash occur at the first file not found or at the second one? — Steve, Apr 24 '13 at 20:34
First of all: Second: Do use "using", as suggested already. Second: Do NOT use Environment.Exit. It terminates the program very hard without closing all resources which are in "using"-scope. — Martin Mulder, Apr 24 '13 at 20:34
`Application.Exit` is safer and properly closes all resources. What is your `Print()` and `LogClose()` method definition? — Cameron Tinker, Apr 24 '13 at 20:40
@cameron tinker: it's a console application. can't use Application.Exit — John Ryann, Apr 24 '13 at 20:47
@CameronTinker `Application.Exit()` is for Windows Forms apps, but I see lots of `Console.WriteLine()` in the OP, so I think it's probably a Console app. — Matthew Watson, Apr 24 '13 at 20:49
@MatthewWatson It didn't occur to me that `Application.Exit` is only for Windows Forms. Thanks for the correction. — Cameron Tinker, Apr 24 '13 at 20:55
Now with your latest code addition, your class should implement `IDisposable`. — Jesse C. Slicer, Apr 24 '13 at 21:28

score 2 · Answer 1 · answered Apr 24 '13 at 21:06

If I was you, I'd be using Log4Net rather than reinventing the wheel. One thing that log4Net gives you is the ability direct log messages to multiple data stores at once. Configure a ConsoleAppender and either a FileAppender or RollingFileAppender and you get console output as well as logfile output for the price of one log.Warn() call.

However, if you're going to roll-yer-own, the point of a log file is to log things in a robust manner: if you leave the underlying TextWriter and its Stream open for the duration without flushing unwritten buffers on every write(), you're likely to lose data in the event of a crash. To that end, a class like this should do you. It opens/closes the file on each write, making sure that things are left in an orderly state:

class MyLogger
{
  private string LogFilePath { get ; set ; }
  public MyLogger( string fileName )
  {
    this.LogFilePath = fileName ;
    return ;
  }

  public void Print( string format , params object[] args )
  {
    using ( TextWriter logfile = new StreamWriter( "" , true , Encoding.UTF8 ) )
    {
      WriteLine( logfile , false , format , args ) ;
    }
    return ;
  }

  private void WriteLine( TextWriter writer , bool echoToConsole , string format , params object[] args )
  {
    string   message = string.Format( format , args ) ;
    DateTime dtNow   = DateTime.Now ;
    const string logFormat = "{0:yyyy-MM-dd hh:mm:ss.ttt}: {1}" ;
    writer.WriteLine( logFormat , dtNow , message ) ;
    if ( echoToConsole )
    {
      Console.WriteLine( logFormat , dtNow , message ) ;
    }
    return ;
  }

  public void ExitAndOut( string format , params object[] args )
  {
    using ( TextWriter logfile = new StreamWriter( "" , true , Encoding.UTF8 ) )
    {
      WriteLine( logfile , true , format , args ) ;
      WriteLine( logfile , true , "Program Exit" ) ;
    }
    Environment.Exit(1) ;
    return ; // unreachable code
  }

}

score 1 · Answer 2 · answered Apr 24 '13 at 20:37

1

you need to rewrite the code to something more like this (pseudo):

public static void Parse( )
{
   using( var sw = new StreamWriter(@"c:\file.txt") )
   { 
       foreach (string sf in ff)
       {
           if (File.Exists(sf))
              File.Move(sf, copy_dir + "\\" + sf);
           else
              Print( sw, "Error: " + sf + " not found");
       }
   }
}

public static void Print(StreamWriter s, string log)
{
    DateTime ts = DateTime.UtcNow;
    s.WriteLine(ts + " " + log);
}

answered Apr 24 '13 at 20:37

Timmerz

6,090
5
36
49

the StreamWriter has to be in logger class. pls see my edited code – John Ryann Apr 24 '13 at 20:45
just use something like link below...but the point is still the same -- the way you are writing your code is not well designed...it's very functional style. http://stackoverflow.com/questions/5057567/how-to-do-logging-in-c – Timmerz Apr 24 '13 at 20:48
2

if the `StreamWriter` is in a logger class then that class can implement IDisposable and handle closing the resources and still do a `using` or `.Close()` on the logger class, or the file can be closed on every write, but I would recommend using an external logging library. these are well travelled paths in programming that avoid issues like you have. – Timmerz Apr 24 '13 at 20:52

score 1 · Answer 3 · answered Apr 24 '13 at 20:50

1

If you are closing the StreamWriter, then you have to open it.

class logger {

public void LogOpen()
{
    StreamWriter sw = new StreamWriter("SpireCli.log");
}

public void Print(string log)
{
    DateTime ts = DateTime.UtcNow;
    sw.WriteLine(ts + " " + log);
}

public void ExitAndOut(string log)
{
    string Program_Exit = "Program Exit";

    Console.WriteLine(log);
    Console.WriteLine(Program_Exit);
    LogOpen();
    Print(log);
    Print("Exit");
    LogClose();
    Environment.Exit(1);
}

public void LogClose()
{
    sw.Close();
}
}

since StreamWriter implements IDisposable, better way is you should use using statement.

answered Apr 24 '13 at 20:50

Kishore Kumar

12,675
27
97
154

sw is not closed. if it were the program would exit. this is really puzzling. Print() is clearly called *before* LogClose – John Ryann Apr 24 '13 at 20:55
Since you are using foreach, when it comes to the second iteration, the sw is closed, and you are using the same object of logger. – Kishore Kumar Apr 24 '13 at 20:56
Its better to debug the foreach, I think, the first statement will work fine, and when it comes to the second, it will throw exception – Kishore Kumar Apr 24 '13 at 20:57
but when the first statement comes, the program exit and never execute the second statement from the loop. the error is from first loop – John Ryann Apr 24 '13 at 20:59
@TheIndianProgrammmer It'll never call the `Close` twice because of the `Environment.Exit` no matter how many iterations of the `foreach` execute. The failed condition happens once and only once and the program terminates. – Jesse C. Slicer Apr 24 '13 at 21:31

C#: Cannot write to a closed TextWriter

3 Answers3