2

I created a function to make sure an object is disposed of properly. This function includes setting the object to null. I am wondering if the line that sets the object to null is useless (and hence I will remove the line), and then add a line to set the object to null in the calling function. My example is for the FileStream object, but I any other object (I think) can take its place. I know I can trace the execution of the program and see what is happening, however, I would like to know more information on the inner mechanisms (garbage collection?), does this work for any object, etc.

//Called function:
public static void DiscardFile(System.IO.FileStream file)
{
    file.Flush();
    file.Close();
    file.Dispose();
    //Does this work?
    //When the function returns, is the file object really set to null?
    file = null;
}

//Calling function:
public static void WriteStringToFile(string s, string fileName)
{
    System.IO.StreamWriter file = new System.IO.StreamWriter(fileName);
    file.Write(s);
    DiscardFile(file);
    //Is this redundant?
    //Or is the line in the called function the redundant line?
    file = null;
}

Thanks!

I have a loop that writes a thousand strings to files within 30 seconds. (The program will be writing 400K+ strings when it completes its execution.) I see that the loop waits (every so often) at the file.Write(s) line, and that the memory footprint of the app increases. That is for another thread, but wanted to know the behavior of the above code.

Thanks!

  • 4
    Don't waste your time setting a parameter or a local variable to `null`. It's already going out of scope when you exit the method. – 15ee8f99-57ff-4f92-890c-b56153 May 21 '19 at 14:56
  • 3
    I suggest you read https://jonskeet.uk/csharp/parameters.html - changing the value of a value parameter (e.g. your `file = null;`) doesn't change anything in the calling code. Not that you need to set variables to null anyway... – Jon Skeet May 21 '19 at 14:56
  • 3
    Also, for your concrete example - just use `File.WriteAllText`. – Jon Skeet May 21 '19 at 14:56
  • 1
    They're both pointless. – SLaks May 21 '19 at 14:57
  • 4
    None of what you're doing actually *disposes* anything. Read up on the `using` statement. – Jeroen Mostert May 21 '19 at 14:58
  • 1
    `using (var file = new System.IO.StreamWriter(fileName)) {...}` – Dmitry Bychenko May 21 '19 at 14:58
  • 1
    @JeroenMostert He does call `file.Dispose();` (edit: in the wrong scope), but yes, he should be using `using` instead. – 15ee8f99-57ff-4f92-890c-b56153 May 21 '19 at 14:58
  • 1
    Do use `using` statement which will call `Dispose` implicitly, even if the writer throws an exception. – Adwaenyth May 21 '19 at 14:59
  • If you want information about garbage collection that is both accurate and way more then you're ever likely to need, check out [this](https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/fundamentals). If you want the part that you'll need to know and understand every day, [this answer](https://stackoverflow.com/a/1480897/5101046) sums it up in a few paragraphs. It also mentions `using` and `Dispose`. – Scott Hannen May 21 '19 at 15:07
  • Jon-- File.WriteAllText <-- thanks! – magalabastro May 21 '19 at 15:09
  • 1
    When you say `file = null;`, you aren't _"setting the object to null"_, you are _setting the variable to null_. An object (an instance of a reference type) can have zero, one or more variables that refer to it at any time. When the number of references drops to zero, the object _becomes eligible for garbage collection_. As others have pointed out, `file` is about to out of scope, so setting it to `null` does nothing. Read up on reference types, garbage collection, the `Dispose` pattern and the `using` statement. – Flydog57 May 21 '19 at 15:09
  • Flydog-- excellent explanation. Follow-up Q: when I have two pointers A&B to an object, I dispose the object using variable A. Is A null at that point? Is B also null? Thanks! – magalabastro May 21 '19 at 16:27
  • @magalabastro Neither. Calling a method on a reference will not set that reference to null (you can test this yourself). Calling Dispose() executes whatever code is in the Dispose() method, that's all. If you want to set a reference to `null`, do so explicitly. But as said above, if it's a parameter or a local, don't bother setting it to null to make GC happen, because that reference is about to go away anyhow when you leave that scope. – 15ee8f99-57ff-4f92-890c-b56153 May 21 '19 at 20:12
  • @EdPlunkett - Thanks! – magalabastro May 23 '19 at 15:21

1 Answers1

2

Sorry, but your implementation is dangerous

public static void WriteStringToFile(string s, string fileName)
{
    System.IO.StreamWriter file = new System.IO.StreamWriter(fileName);
    file.Write(s);          // <- the danger is here                   
    DiscardFile(file);
    //Is this redundant?                                        Yes, it's redundant
    //Or is the line in the called function the redundant line?
    file = null;
}

Suppose you have an exception thrown on file.Write(s); it means that DiscardFile(file); will never be executed an you have resource leakage (HFILE - opened file handle). Why not stick to standard using pattern:

public static void WriteStringToFile(string s, string fileName) 
{
    // Let system release all the resources acquired 
    using var file = new System.IO.StreamWriter(fileName); 
    {
        file.Write(s);
    }  // <- here the resources will be released
}

In case of C# 8.0 you can get rid of pesky {...} and let the system release resources on leaving method's scope (see https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-8#using-declarations):

public static void WriteStringToFile(string s, string fileName) 
{
    // Let system release all the resources acquired 
    using var file = new System.IO.StreamWriter(fileName); 

    file.Write(s);
} // <- here the resources will be released
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215