Where Did My Object Go? Part 2

In Part 1 of this article I discussed the possibility of an object instance being collected before a method returns in this scenario:

    new MyObject().LongRunningMethod();

as well as this scenario:

    MyObject o = new MyObject();
    o.LongRunningMethod();

Here we'll discuss how this could become a problem. Frankly, it's pretty easy to cause the problem, but I think it generally involves some ugliness on the part of software design or implementation, largely involving clean-up of fields when the instance is finalized.

Don't Try This at Home

I don't think it's likely you'll see a lot of code that does this, but here's one way to run afoul of your object going way: export a reference to a field that the object cleans up when it's finalized. You can export the reference, say, by making that field visible through a property. Exposing a field that you're going to clean up in the finalizer would be an easy way to create a coupling between your object and other (arbitrary) client code that has no knowledge of your object's life span.

Call Stack Antics

Another way to invoke the potential problem is to lose your "this" reference on the call stack. The code below manages to lose the "this" reference by passing a field to the helper method rather than allowing the helper to access the field directly via its own "this" reference. When the helper tries to access fs.Length an ObjectDisposedException is thrown.

Why does this throw? Well, the last live reference to the instance was lost when LongRunningMethod passed _input to Helper. Essentially we've again exported the field value from the instance and no longer hold a reference to the instance, allowing the GC to finalize it. Helper is left holding a reference to an object that has been finalized.

Note: Again, you will not see this behavior in a debug build. When running code marked as "debug" the JITter extends the lifetime of the local object to the end of the method. So you will not see this effect if you've compiled with the /debug flag.

    using System;
    using System.IO;

    // Ugliness ensues.
    sealed class MyUglyObject
    {
        public MyUglyObject(string inputPath)
        {
            // Real production code would likely not delete the file when done...
            // ...but this is a sample app.
            _input = new FileStream(Path.GetTempFileName(), FileMode.Open, FileAccess.Read, FileShare.Read);
        }

        ~MyUglyObject()
        {
            if (_input != null)
            {
                _input.Close();
                _input = null;
            }
        }

        public void LongRunningMethod()
        {
            Helper(_input); // Our last reference to 'this' (implicit).
        }

        private void Helper(FileStream fs)
        {
            // A long-running method can easily experience a garbage collection 
            // before returning. This one happens for force it to occur.
            GC.Collect();
            GC.WaitForPendingFinalizers();

            // Ka-boom!
            long inputSize = fs.Length;
            // ...
        }

        private FileStream _input;
    }

    class Program
    {
        static void Main(string[] args)
        {
            new MyUglyObject(@"..\..\readme.txt").LongRunningMethod();
        }
    }

Can You See It?

As you can see above it takes a bit of effort to cause the code to blow up. If Helper had used _input.Length instead of taking a parameter the problem would not exist.

But, what I find a bit creepy about the above code is that if Helper were a static method it would seem respectable:

    private static void Helper(FileStream fs)
    {
        // A long-running method can easily experience a garbage collection 
        // before returning. This one happens for force it to occur.
        GC.Collect();
        GC.WaitForPendingFinalizers();

        // Ka-boom!
        long inputSize = fs.Length;
        // ...
    }

At first glance it now looks like helper is a normal static helper function, as is likely seen in code bases across the world. It doesn't need a "this" reference, it takes a reference to the object it uses, everything appears fine on the surface. Would you see the "this" reference being lost by the code calling Helper in a code review? I'm not so sure I would have until recently.

 

 

Technorati tags: Coding, Software Design, CLR, C#, Code Review