0

Please have a look at sample code given below. I have two questions:
1. Do we need to close data reader before returning something or GC will take care of it?
2. Do we need to set custom class objects as null or GC will take care of it in these two scenarios?

public class Database
{
    public string DoSomething()
    {
        using (SqlConnection con = new SqlConnection(connectionString))
        {
            using (SqlCommand cmd = new SqlCommand("Select ID,Name From Person", con))
            {
                SqlDataReader reader = cmd.ExecuteReader();

                while(reader.Read())
                {
                    return reader["Name"].ToString();
                    //will this close reader once string is returned or we have to close it?
                }
            }
        }
    }
}

USAGE

Database database = new Database();
string res = database.DoSomething();
database = null; //is it necessary to do this or GC will take care of it?

Another class

public class XML
    {
        public void ReadXML()
        {
            XmlDocument doc = new XmlDocument();
            doc.Load("somefile.xml");
        }
    }

USAGE

XML xml = new XML();
xml.ReadXML();
xml = null; //is it necessary to do this or GC will take care of it?
Frank Martin
  • 3,147
  • 16
  • 52
  • 73

1 Answers1

2

TL;DR You are missing a using statement at the SqlReader. The other scenrarions are handled by the GC. Always use Dispose or using if you are done with objects that implements the IDisposable interface (there are a few exceptions on this last note).

note: for a "subtitle" difference in lifetime and scope, see @Damien_The_Unbeliever's comment. For more info about the garbage collection see MSDN: https://msdn.microsoft.com/en-us/library/0xy59wtx(v=vs.110).aspx

In general, all managed objects which have no outstanding references, will be taken care of by the GC. So in general, you don't need to explicitly set them to null.

However, if a object implements the IDisposable interface, it is preferred to call Dispose when you are done with the object. Although (if correctly implemented), this will also be called during the Finalize call, it's preferred to call Dispose explicit (or through the using statement). Objects, like database connections, graphic objects or network-connection can otherwise lead to a full connection pool or occupies graphic memory.

Be advised that this is all about scope and outstanding references.

if you have a method, like this, the object "behind" databasewill be taken care of as soon as it leaves the scope because no other variables are referring to this object:

public void Foo()
{
   Database database = new Database();
   string res = database.DoSomething();
   //you don't need this call 
   // because the will be no outstanding references after the method exits.
   //database = null;
}

But if the containing object is static or kept alive in another way, you'll get this scenario; the database is kept alive as long as there is a valid reference somewhere to that FooClass object:

public class FooClass
{
    Database database = new Database();

    public void Foo()
    {  
        //some operation
    }    
}

If a member implements IDisposable it is best to call it through a using statement or explicitly (Dispose()). Although the GC should handle this, especially with third party libraries, it requires for IDisposable to be correctly implemented. If you see this example, you can see that it can be tricky and therefore is a suspect when facing memory leaks.

Proper use of the IDisposable interface

So, to answer your question's scenarios:

In the following, the using statement, is equivalent to a try, catch finaly block. See https://learn.microsoft.com/en-us/dotnet/articles/csharp/language-reference/keywords/using-statement

public class Database
{
    public string DoSomething()
    {
        using (SqlConnection con = new SqlConnection(connectionString))
        {
            using (SqlCommand cmd = new SqlCommand("Select ID,Name From Person", con))
            {
                //note: using a using here as well
                using (SqlDataReader reader = cmd.ExecuteReader())
                {
                    while(reader.Read())
                    {
                        //this is ok, because the USING will take care of the disposure.
                        return reader["Name"].ToString();
                    }
                }
            }
        }
    }
}

So far I think this is enough information to answer the question about the GC. There is one point of attention though; if you are facing unmanaged objects, there are other calls necessary for proper disposure. COM for example requires Marshall.ReleaseComObject(), and some libraries (some industrial camera drivers I know of), require an explicit, CleanUp call.

Community
  • 1
  • 1
Stefan
  • 17,448
  • 11
  • 60
  • 79
  • 1
    For your first example, I think you confuse scope and lifetime. Provided you're running in release mode, the JIT compiler and GC collude to understand actual lifetimes. If your commented section actually contains lots of time consuming code but doesn't otherwise make use of `database`, it may be collected whilst `Foo` is still running. Similarly, an object can be collected *whilst its constructor is still running*, provided no actual accesses are potentially in the future. – Damien_The_Unbeliever May 16 '17 at 19:30
  • Good point; I'll make an edit and reference to your comment. I think my effort to keep it simple introduced some inaccuracies. – Stefan May 16 '17 at 19:42