Using clause fails to call Dispose?

Question

I'm using Visual Studio 2010 to target .NET 4.0 Client Profile. I have a C# class to detect when a given process starts/terminates. For this the class uses a ManagementEventWatcher, which is initialised as below; query, scope and watcher are class fields:

query = new WqlEventQuery();
query.EventClassName = "__InstanceOperationEvent";
query.WithinInterval = new TimeSpan(0, 0, 1);
query.Condition = "TargetInstance ISA 'Win32_Process' AND TargetInstance.Name = 'notepad.exe'";

scope = new ManagementScope(@"\\.\root\CIMV2");

watcher = new ManagementEventWatcher(scope, query);
watcher.EventArrived += WatcherEventArrived;
watcher.Start();

The handler for event EventArrived looks like this:

private void WatcherEventArrived(object sender, EventArrivedEventArgs e)
{
    string eventName;

    var mbo = e.NewEvent;
    eventName = mbo.ClassPath.ClassName;
    mbo.Dispose();

    if (eventName.CompareTo("__InstanceCreationEvent") == 0)
    {
        Console.WriteLine("Started");
    }
    else if (eventName.CompareTo("__InstanceDeletionEvent") == 0)
    {
        Console.WriteLine("Terminated");
    }
}

This code is based on a CodeProject article. I added the call to mbo.Dispose() because it leaked memory: about 32 KB every time EventArrived is raised, once per second. The leak is obvious on both WinXP and Win7 (64-bit).

So far so good. Trying to be conscientious I added a try-finally clause, like this:

var mbo = e.NewEvent;
try
{
    eventName = mbo.ClassPath.ClassName;
}
finally
{
    mbo.Dispose();
}

No problem there. Better still, the C# using clause is more compact but equivalent:

using (var mbo = e.NewEvent)
{
    eventName = mbo.ClassPath.ClassName;
}

Great, only now the memory leak is back. What happened?

Well, I don't know. But I tried disassembling the two versions with ILDASM, which are almost but not quite the same.

IL from try-finally:

.try
{
  IL_0030:  nop
  IL_0031:  ldloc.s    mbo
  IL_0033:  callvirt   instance class [System.Management]System.Management.ManagementPath [System.Management]System.Management.ManagementBaseObject::get_ClassPath()
  IL_0038:  callvirt   instance string [System.Management]System.Management.ManagementPath::get_ClassName()
  IL_003d:  stloc.3
  IL_003e:  nop
  IL_003f:  leave.s    IL_004f
}  // end .try
finally
{
  IL_0041:  nop
  IL_0042:  ldloc.s    mbo
  IL_0044:  callvirt   instance void [System.Management]System.Management.ManagementBaseObject::Dispose()
  IL_0049:  nop
  IL_004a:  ldnull
  IL_004b:  stloc.s    mbo
  IL_004d:  nop
  IL_004e:  endfinally
}  // end handler
IL_004f:  nop

IL from using:

.try
{
  IL_002d:  ldloc.2
  IL_002e:  callvirt   instance class [System.Management]System.Management.ManagementPath [System.Management]System.Management.ManagementBaseObject::get_ClassPath()
  IL_0033:  callvirt   instance string [System.Management]System.Management.ManagementPath::get_ClassName()
  IL_0038:  stloc.1
  IL_0039:  leave.s    IL_0045
}  // end .try
finally
{
  IL_003b:  ldloc.2
  IL_003c:  brfalse.s  IL_0044
  IL_003e:  ldloc.2
  IL_003f:  callvirt   instance void [mscorlib]System.IDisposable::Dispose()
  IL_0044:  endfinally
}  // end handler
IL_0045:  ldloc.1

Apparently the problem is this line:

IL_003c:  brfalse.s  IL_0044

which is equivalent to if (mbo != null), so mbo.Dispose() is never called. But how is it possible for mbo to be null if it was able to access .ClassPath.ClassName?

Any thoughts on this?

Also, I'm wondering if this behaviour helps explain the unresolved discussion here: Memory leak in WMI when querying event logs.

Answer 1

At first glance, there appears to be a bug in ManagementBaseObject.

Here's the Dispose() method from ManagementBaseObject:

    public new void Dispose() 
    {
        if (_wbemObject != null) 
        {
            _wbemObject.Dispose();
            _wbemObject = null;
        } 
        base.Dispose();
        GC.SuppressFinalize(this); 
    } 

Notice that it is declared as new. Also notice that when the using statement calls Dispose, it does so with the explicit interface implementation. Thus the parent Component.Dispose() method is called, and _wbemObject.Dispose() is never called. ManagementBaseObject.Dispose() should not be declared as new here. Don't believe me? Here's a comment from Component.cs, right above it's Dispose(bool) method:

    ///    <para>
    ///    For base classes, you should never override the Finalier (~Class in C#) 
    ///    or the Dispose method that takes no arguments, rather you should 
    ///    always override the Dispose method that takes a bool.
    ///    </para> 
    ///    <code>
    ///    protected override void Dispose(bool disposing) {
    ///        if (disposing) {
    ///            if (myobject != null) { 
    ///                myobject.Dispose();
    ///                myobject = null; 
    ///            } 
    ///        }
    ///        if (myhandle != IntPtr.Zero) { 
    ///            NativeMethods.Release(myhandle);
    ///            myhandle = IntPtr.Zero;
    ///        }
    ///        base.Dispose(disposing); 
    ///    }

Since here the using statement calls the explicit IDisposable.Dispose method, the new Dispose never gets called.

EDIT

Normally I would not assume that something like this a bug, but since using new for Dispose is usually bad practice (especially since ManagementBaseObject is not sealed), and since there is no comment explaining the use of new, I think this is a bug.

I could not find a Microsoft Connect entry for this issue, so I made one. Feel free to upvote if you can reproduce or if this has affected you.

Answer 2

This issue also causes MS Unit Test Framework to fail and hang forever at the end of running all tests (under Visual Studio 2015, update 3). Unfortunately the bug still persists as I am writing this. In my case the following code is leaking:

using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(query))
{
    ....
}

And what Test Framework is complaining about is about a thread not being shutdown:

System.AppDomainUnloadedException: Attempted to access an unloaded AppDomain. This can happen if the test(s) started a thread but did not stop it. Make sure that all the threads started by the test(s) are stopped before completion.

And I managed to get around it by executing the code in another thread (therefore, after the starter thread exits, hopefully all other threads spawned in it are closed and resources are freed appropriately):

Thread searcherThread = new Thread(new ThreadStart(() =>
{
    using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(query))
    {
        ....
    }
}));
searcherThread.Start();
searcherThread.Join();

I am not advocating that this is the solution to the problem (as a matter of fact, spawning a thread just for this call is a horrible idea), but at least I can run tests again without needing to restart Visual Studio every time it hangs.

Answer 3

We are seeing similar problem,

calling GC.WaitForPendingFinalizers() once is enough to fix the leak

though I am aware it is not a solution, just a workaround the bug