Growing IDisposable Guided By Tests

Uncategorized
Test 11: ObjectDisposedException

ObjectDisposedException should be thrown if Dispose() has been called and an attempt is made to call a public method that needs to access the finalized resources:

[Fact]
public void ThrowsObjectDisposedExceptionIfDisposedResourcesAccessed()
{
    var resourceOwner = new ResourceOwner();
    resourceOwner.Invoking(r => r.PublicMethodThatUsesFinalizableResources())
                 .ShouldNotThrow<ObjectDisposedException>();
    resourceOwner.Dispose();
    resourceOwner.Invoking(r => r.PublicMethodThatUsesFinalizableResources())
                 .ShouldThrow<ObjectDisposedException>();
}

To make this compile we add a public method that accesses the finalized resources:

public class ResourceOwner : IDisposable
{
    ...
    public void PublicMethodThatUsesFinalizableResources()
    {
        Marshal.PtrToStringAuto(AnUnmanagedResource);
        ADisposableResource.ReadByte();
    }
}

The test passes! This is a bad sign – we must always see a test fail to know that it is our code changes that make it ultimately pass. MemoryStream throws the ObjectDisposedException when we call ReadByte() after it has been disposed. The exception bubbles up and the test passes for the wrong reason. We need to remove this call to force ResourceOwner to change:

public class ResourceOwner : IDisposable
{
    ...
    public void PublicMethodThatUsesFinalizableResources()
    {
        Marshal.PtrToStringAuto(AnUnmanagedResource);
    }
}

Now the test fails. To make it pass:

public void PublicMethodThatUsesFinalizableResources()
{
    if (_disposed) throw new ObjectDisposedException(GetType().FullName);
    Marshal.PtrToStringAuto(AnUnmanagedResource);
}

The test passes.

 

The completed solution

The complete ResourceOwner, refactored for clarity, now looks like this:

public class ResourceOwner : IDisposable
{
    protected IntPtr AnUnmanagedResource = Marshal.StringToCoTaskMemAuto("Foo");
    protected MemoryStream ADisposableResource = new MemoryStream();
    private bool _disposed;
 
    ~ResourceOwner()
    {
        Dispose(false);
    }
 
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
 
    protected virtual void Dispose(bool disposing)
    {
        if (_disposed) return;
        if (disposing)
        {
            DisposeDisposableResources();
        }
        FreeUnmanagedResources();
        _disposed = true;
    }
 
    public void PublicMethodThatUsesFinalizableResources()
    {
        if (_disposed) throw new ObjectDisposedException(GetType().FullName);
        SomeCodeThatUsesFinalizableResources();
    }
 
    private void FreeUnmanagedResources()
    {
        Marshal.FreeCoTaskMem(AnUnmanagedResource);
    }
 
    private void DisposeDisposableResources()
    {
        ADisposableResource.Dispose();
    }
 
    private void SomeCodeThatUsesFinalizableResources()
    {
        Marshal.PtrToStringAuto(AnUnmanagedResource);
    }
}

 

And all the tests pass :)

The complete source code can be found here.

Pages: 1 2 3 4 5

2 thoughts on “Growing IDisposable Guided By Tests”

  1. Regarding test 3, you could check your call to GC.SuppressFinalize(this) by introducing a non-public instance method SuppressFinalizeOnThis() which delegates to GC. This then acts like a collaborator that, in some languages, you might pull out into a mixin/trait.

    I don’t think I’d suggest it until you had reason to believe that you weren’t calling GC.SuppressFinalize() correctly.

    1. Hi J.B. Thanks for the reply.

      I thought about doing something like that but it just seems to shift the problem (if I am understanding you correctly). While this would allow me to test that SuppressFinalizeOnThis() was called, the thing I really need to test is that GC.SuppressFinalize(this) was called. I would still need to confirm that this call was being made inside SuppressFinalizeOnThis(). I don’t know how to do that except by visual inspection, which gets me back to the original problem.

Comments are closed.