views:

27

answers:

2

Hi. I have the following code: A disposable class that contains two disposable members. One of them is being initialized using new() method, and the other using static factory method. I Also have static code analisys rules, with CA2213 as error.

    public class DisposableClass : IDisposable
{
    private WebClient m_DisposableMember1;

    public WebClient DisposableMember1
    {
        get
        {
            if (m_DisposableMember1 == null)
            {
                m_DisposableMember1 = new WebClient();
            }
            return m_DisposableMember1;
        }
    }

    private WebClient m_DisposableMember2;

    public WebClient DisposableMember2
    {
        get
        {
            if (m_DisposableMember2 == null)
            {
                m_DisposableMember2 = Factory.Create();
            }
            return m_DisposableMember2;
        }
    }


    #region Finalize/Dispose Pattern
    private bool m_IsDisposed = false;

    //Implement IDisposable.
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~DisposableClass()
    {
        Dispose(false);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!m_IsDisposed)
        {
            if (disposing)
            {
                DisposableMember1.Dispose();
                // DisposableMember2 in not disposed and not notified by fxCop
            }

            m_IsDisposed = true;
        }
    }
    #endregion Finalize/Dispose Pattern

}

This is the simple factory class:

    public static class Factory
{
    public static WebClient Create()
    {
        return new WebClient();
    }
}

When I call the Dispose() method of the DisposableMember1 property, I get CA2213. When I call the Dispose() method of the m_DisposableMember1 member, I don't get this error.

More over, I don't get this error for m_DisposableMember2 (Wich was initialized using the static factory), and it is not being disposed.

Does anybody familiar with this issue? What can cause this behavior?

+1  A: 

CA2213 is not particularly aggressive about identifying fields that ought to be disposed. Amongst other potential problems, it only considers fields that are directly assigned from a constructor invocation. Assignment of a field value via a property or from a source other than a constructor invocation do not result in the target field being included in the pool of "should be disposed" fields by the rule.

Nicole Calinoiu
+1  A: 

You are expecting a bit too much out of the smarts of this tool. That the property is an alias for the field is only obvious to human eyes. It requires pretty sophisticated analysis to let a tool see that. FxCop just doesn't have that kind of horse power. Your code is in fact questionable. If the property was never used by the client code then you'll create the WebClient object and immediately dispose it. Use the field instead, test for null and Dispose().

The problem with the second field is probably induced by the need for the tool to avoid unnecessary warnings. It will only complain if it can determine with absolute certainty that the field is ever initialized. Same kind of lack of smarts here, it can't tell that the factory always returns a non-null reference. Only when it sees the constructor being used in the class itself can it be sure that the field needs to be disposed.

It's a good tool, not perfect.

Hans Passant