Best practice while writing C# functions

Hi,
Anyone has any idea about how to free up memory in C# functions. Does the GC collects memory after a function has executed How about setting variables to null after we are finished with them, or the GC will automatically set them to null Does setting variables to null in finally causes an overhead

void TestFunc()
{
XmlDocument xdDoc = new XmlDocument();
try
{
// use xdDoc
}
catch
{
// handle catch
}
finally
{
xdDoc = null;
}

}



Answer this question

Best practice while writing C# functions

  • Tony Chun Tung Siu

    I'm sorry, but no bug in the XmlDocument has been demonstrated in this thread. For anyone in the slightest doubt whether or not XmlDocument is garbage collected like any other object, there is a simple way to test. The class is not sealed. So simply derive a new class from XmlDocument and implement IDisposable. Create a test program that instantiates your new document class and perhaps load some xml into it, then lets the variable go out of scope without assigning null to it or calling dispose.

    Set a breakpoint in the Dispose method and run the test program with the debugger attached. As you'll see, you hit the breakpoint despite your code never calling Dispose - because the garbage collector calls it before freeing (and compacting) managed memory.

    Then modify the code and assign a reference to any node in the document to a static field. Run again. Since your code doesn't remove the reference, Dispose will not be called until the program exits.

    Anyone who thinks there should be a way to explicitly free memory do not understand the advantages of garbage collection. "Freeing up memory" really means to write a string of zero bytes into some address range in RAM. It does nothing to improve the performance of anything, although it does please people who don't know any better to see in task manager that they are using little RAM (as if somehow the PC runs faster when you use little RAM -- of course it does not).

    Something similar has to be said about the idea that all types should be disposable. This is so far from the truth that it must be based on serious misconceptions about what the idea behind disposable objects is. The *ONLY* time it makes any sense for a type to be disposable is when unmanaged resources are involved. For example, any object that uses platform invokes and aquires windows handles should be made disposable to help ensure handles are released. But in a fully managed implementation like XmlDocument there simply is nothing useful or sensible that could be done in the Dispose() method, and consequently there is no need to implement it.

    I'm sorry if I step on anyone's toes, but I become somewhat annoyed when people who do not have any understanding of how things work insist on spreading their own misconceptions to other people. It's not right, and somebody should say so clearly to battle the resulting confusion.

    The real problem with the garbage collector approach to managing memory is that it doesn't take into account the memory needs of unmanaged processes. It is possible that an unmanaged memory allocation fails when physical memory is available, because managed memory that is eligible for garbage collection will not be freed as a result of attempting to allocate unmanaged memory. Also, the .NET 1.x garbage collector has some limitations regarding "large objects" that can lead to memory fragmentation over time in some scenarios. (The collector defragments memory by moving objects to new locations and updating all references accordingly to ensure that big enough continuous chunks of free memory are available for object allocations, but since copying a large block of memory takes longer it does not move what it considers to be "large objects" - the limit in 1.x was 16K if I recall correctly.) Although it would obviously incur a performance hit, it would be nice if the runtime could be configured to compact (defragment) all memory including that used by "large objects" as sometimes the performance hit would be perfectly acceptable.


  • Hajimu Takahashi

    Here is what I do :
    For reference variables, if the type implements IDisposable then I call the Dispose method and then set it to null. I do this for both local variables and fields although setting a local reference variable to null doesnt do anything much. If you dont set a field to null after using it, the garbage collector cannot reclaim it (assuming a garbage collection occurs while your application is running). If the garbage runs only when your application ends, it doesnt make much difference anyway. I hold the stand that setting a variable to null after you are done with it is a good programming practice. If that variable is used again you get a NullReferenceException which shows there is a bug somewhere.
    For value types I wouldnt bother doing anything because the memory is reclaimed when the stack unwinds.



  • anand_nalya

    AI Sam,

    Can you show me a repro for the XmlDocument leak Unless the XmlDocument is setting itself to a static field somewhere, once it falls out of scope and as long as all of its XmlNodes are also out of scope, then it will become eligible for collection.

    As Phil has already stated above, there is never any need to null out a method's local variables*. However, there may be some benefit to nulling out large member fields in an object's dispose method, like so (assume below that we've implemented the Disposable pattern correctly):



    public A: IDisposable
    {
       private B _Member;
     
       [...] 
     
       protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
                if (_Member != null)
                {
                    _Member.Dispose();
                    _Member = null;
                }
            }
        }
    }

     

    That way, if someone continues to hold a reference to A after it has been disposed, _Member can still be collected.

    *Perhaps if you passing a parameter by reference, as dagjo mentioned, but that seems to me to be a questionable pattern for the method to be responsible for setting the external reference to null.



  • davidsbuchan

    Dexter.Net wrote:

    As more and more memory allocations are required, the least used objects are moved into higher generations of the heap. The most frequently used objects would be maintained in generation 0 of the heap.

    How frequently you use an object has nothing to do with it (how do you define use anyway ). The object's lifetime - the number of collections it has survived - determines the generation it's in.

    Dexter.Net wrote:

    To answer your question, the best practice for coders should be use the dispose method under the finally block and set object references to nothing. Though this will not execute the GC, but will at most free up a lot of memory.

    Setting a reference to Nothing will not directly free up any memory.



  • Ashok Ojha

    PhilDWilson wrote:

    The best practice is to never set anything to null.

    I wouldn't go as far as saying that. When you can shorten the lifetime of an object by making it eligible for GC sooner than it otherwise would be, it makes perfect sense to null out a reference to it. That's probably more common with class fields than local variables though.



  • smartpi

    Hi All,
    lots of answers

    Well, I think Microsoft should itself tell developers about these things. Like Al Sam said he has found a bug in XmlDocument, may be it worked that way and no one knew about it. As other types require a Dispose , XmlDocument needs a RemoveAll() to release its nodes.

    I think every collection or encapsulating type must implement something like a dispose, that'll free up memory(or mark) held by its child elements. e.g. XmlDocument's XmlNode. Because as of today we have no means of freeing memory explicitly in .NET.


  • Luke Westendorf MSFT

    The best practice is to never set anything to null. The runtime is pretty smart. Build and run this in release mode (because debug mode behaves differently).

    using System;

    namespace Final
    {
    class Class1
    {
    [STAThread]
    static void Main(string[] args)
    {
    Thingy q = new Thingy();
    GC.Collect();
    q = new Thingy();
    Console.ReadLine();
    }
    }
    class Thingy
    {
    ~Thingy()
    {
    Console.WriteLine("gone");
    }
    }
    }

    That call on GC.Collect() is completely unnecessary, but it's there to illustrate that the object is deallocated and the Finalizer is called when GC.Collect is called, despite the fact that there appear to be references to q later in the code. The runtime knows that the Thingy q created on the first line is available for collection as soon as it's been built, maybe even before ;=). So don't think of deallocation as being related to identifiers in your code or scope or curly braces, think of whether the actual object has any viable references to it. In your sample code, xdDoc is available for collection as soon as nothing can refer to it, most likely somewhere in your try block. Setting it to null in the finally has no effect.



  • Gustavo.

    An object becomes available for garbage collection once there are no longer reachable references to that object.  Local variables which reference the object are destroyed once the function exits, so there is no point in setting them to null.  In fact, the compiler may very well optimize these out.

    In the example you gave, assuming the xdDoc is the only variable that references the XmlDocument object, the XmlDocument object will be eligible for collection as soon as the function exits, whether you set xdDoc to null or not.  The GC will determine when it actually collects the XmlDocument object, however.


  • Lyn Perkins


    While using the Dispose() method implementations and relying on the garbage collector is the accepted best practice for memory management in .NET, when dealing withe the XmlDocument class, I have found a memory issue - which may be a bug in the 1.1 version of the .NET Framework...

    XmlDocument has no Dispose() method, and event after it passes out of scope, the GC does not reclaim its memory. In order to fix this problem, you must call the RemoveAll() method if the XmlDocument class in order for the GC to reclaim the memory used. Otherwise - somewhere, somehow - the XmlNode instances used by XmlDocument will remain referenced out of scope - and thus not collected by the garbage collector until the application is exited.

    --
    Sam Jones
    Adaptive Intelligence
    http://www.adaptiveintelligence.net



  • Ken_Hosinski

    Pradeep C wrote:
    Here is what I do :
    If you dont set a field to null after using it, the garbage collector cannot reclaim it (assuming a garbage collection occurs while your application is running). If the ....

    This is not correct, as you can verify by running the example program, 3rd message in this thread.



  • Jingwei Lu

    Mattias: I agree. The only case I've ever come across was a long-lived object where a field was a large buffer that could be freed by setting it to null, and re-allocated later. As a starting point "never set to null" is a reasonable start, and I would quote the OP's code as my proof! However I can imagine a bunch of people reading your comment and thinking "hmmm, that might apply to me" and then we're back to people unnecessarily setting everything to null all the time. Perhaps I should not have called it the best practice but rather the best default rule to apply, and don't even *think* about breaking it until you know you have memory issues, and then analyze them.

  • rosnay

    XmlNodes have a reference to their document, so if you had a live reference to any of a document's nodes, then the document itself would not be eligible for garbage collection. Calling RemoveAll removes the reference between the node and the document. Could this be why your XmlDocument object wasn't getting collected After all, I find it very hard to believe that Microsoft would leave such a blatant memory leak in a commonly used class of the .NET framework.
  • Marthinus

    To know when the GC actually runs its algorithm to free up the memory, requires that we understand how the GC works at the first place. As we all know, anything created by the framework is maintained on a managed heap. This managed heap is maintained as a multi generation graph. As more and more memory allocations are required, the least used objects are moved into higher generations of the heap. The most frequently used objects would be maintained in generation 0 of the heap. When this generation 0 is completely full, it is at that time the GC algorithm executes to free up memory. The algorithm itself works in two stages where, first, it carries out memory compaction and second, rearranges the heap based on removing the redundant references.

    To answer your question, the best practice for coders should be use the dispose method under the finally block and set object references to nothing. Though this will not execute the GC, but will at most free up a lot of memory.



  • davidhart_home

    Pradeep C's posting is so choke full of errors that I cannot let it stand unopposed.

    1. "setting a local reference variable to null doesnt do anything much."

    If a variable is passed by reference, it isn't really local; passing by reference (e.g. method(ref string s)) means to pass a reference to the variable (whether the variable itself is a value or reference type). Therefore, setting it to null does do rather a lot; it sets the external variable to null.

    2. "If you dont set a field to null after using it, the garbage collector cannot reclaim it"

    Yes it can. For instance members, once the object that has the field itself becomes garbage collectable (i.e. cannot be reached from stack or heap), so does the referenced object (assuming no other references to it exists). The GC doesn't count references - it automatically detects objects that cannot be reached (because no reference to the object exists in the stack or heap, nor in any object that is referenced from there, nor in any object referenced from such objects, and so on...) For an object referenced by a local variable, it becomes garbage collectable when the variable goes out of scope (provided of course there are no other references to the instance).

    3. "If the garbage runs only when your application ends, it doesnt make much difference anyway."

    Technically speaking, if no garbage collection occurs during the lifetime of the application, this would of course be true. However, it is totally misleading because garbage collection isn't so infrequent that it makes sense to think about it on this timescale level. Usually the GC will perform generation 0 collection (that is, it only examines objects that were created after the last collection) several times per second, and particularly so if the application is under stress.

    4. "setting a variable to null after you are done with it is a good programming practice"

    I disagree. Code that doesn't do anything for the program still adds clutter for the developer, not to mention it wears out the keyboard :) When I see four lines at the end of a method that set the local variables to null, it merely tells me the person who wrote the code doesn't understand how garbage collection works in .NET. Any local variable becomes unreachable the moment it goes out of scope.


  • Best practice while writing C# functions