[release/11.0] [System.Management] Make generated WMI wrappers disposable

Project: dotnet/runtime | Language: C# | Concepts: memory-management, backwards-compatibility, api-design | Difficulty: mid
Link: https://github.com/dotnet/runtime/pull/132587

The problem

The strongly-typed WMI collection and enumerator classes generated by System.Management.ManagementClass.GetStronglyTypedClassCode did not implement IDisposable. This prevented consumers from deterministically releasing the underlying COM-backed WMI resources, potentially leading to resource leaks and making it impossible to use these objects in a using statement.

Why it's non-trivial

Simply adding IDisposable to the generated types isn't risk-free:

• The generated types are not sealed, so consumers might have derived from them and implemented their own Dispose method, leading to source or behavioral breaking changes.
• Implementing IDisposable without following the full dispose pattern (including Dispose(bool) and GC.SuppressFinalize) can cause issues for derived types and does not fully align with .NET best practices.
• Care was needed to ensure that the new interface did not break existing consumers or introduce subtle bugs, especially around disposal semantics and inheritance.

The approach

The PR modifies the code generator to add IDisposable to both the generated collection and enumerator classes, and wires up a simple Dispose method that disposes the underlying WMI resource. It also adds tests to verify the new behavior.

1. Add IDisposable to generated collection and enumerator classes:

-            //public class ServiceCollection : ICollection, IEnumerable
+            //public class ServiceCollection : ICollection, IDisposable
             ccc = new CodeTypeDeclaration(PrivateNamesUsed["CollectionClass"].ToString());
 
             ccc.BaseTypes.Add("System.Object");
             ccc.BaseTypes.Add("ICollection");
+            ccc.BaseTypes.Add("System.IDisposable");

This code ensures the generated collection class implements IDisposable in addition to its existing interfaces.

-            //public class ServiceEnumerator : IEnumerator
+            //public class ServiceEnumerator : IEnumerator, IDisposable
             ecc = new CodeTypeDeclaration(PrivateNamesUsed["EnumeratorClass"].ToString());
             ecc.TypeAttributes = TypeAttributes.NestedPublic;
 
             ecc.BaseTypes.Add("System.Object");
             ecc.BaseTypes.Add("System.Collections.IEnumerator");
+            ecc.BaseTypes.Add("System.IDisposable");

Similarly, the generated enumerator class now implements IDisposable.

2. Implement a forwarding Dispose method that disposes the underlying resource:

+            //public void Dispose()
+            //{
+            //    privColObj.Dispose();
+            //}
+
+            cmm = new CodeMemberMethod();
+            cmm.Attributes = MemberAttributes.Public | MemberAttributes.Final | MemberAttributes.Override;
+            cmm.Name = "Dispose";
+            cmm.ImplementationTypes.Add("System.IDisposable");
+            cmie = new CodeMethodInvokeExpression(
+                new CodeVariableReferenceExpression(strObjectCollection),
+                "Dispose"
+                );
+            cmm.Statements.Add(new CodeExpressionStatement(cmie));
+            ccc.Members.Add(cmm);

This adds a Dispose method to the generated collection class, which calls Dispose on the underlying collection object (privColObj).

+            //public void Dispose()
+            //{
+            //    privObjEnum.Dispose();
+            //}
+
+            cmm = new CodeMemberMethod();
+            cmm.Attributes = MemberAttributes.Public | MemberAttributes.Final | MemberAttributes.Override;
+            cmm.Name = "Dispose";
+            cmm.ImplementationTypes.Add("System.IDisposable");
+            cmie = new CodeMethodInvokeExpression(
+                new CodeVariableReferenceExpression(strObjectEnumerator),
+                "Dispose"
+                );
+            cmm.Statements.Add(new CodeExpressionStatement(cmie));
+            ecc.Members.Add(cmm);

Likewise, the enumerator class gets a Dispose method that disposes its underlying enumerator field.

3. Add tests to verify the generated types implement IDisposable and forward disposal:

+        [ConditionalFact(typeof(WmiTestHelper), nameof(WmiTestHelper.IsWmiSupported))]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/34360", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, ".NET Framework uses its inbox System.Management implementation")]
+        public void GeneratedCollectionAndEnumeratorImplementIDisposable()
+        {
+            using var managementClass = new ManagementClass(null, "Win32_LogicalDisk", null);
+            CodeTypeDeclaration classDom = managementClass.GetStronglyTypedClassCode(false, false);
+
+            CodeTypeDeclaration collection = Assert.Single(
+                classDom.Members.OfType<CodeTypeDeclaration>(),
+                type => type.Members.OfType<CodeMemberField>().Any(field => field.Name == "privColObj"));
+            AssertDisposableWrapper(collection, "privColObj");
+
+            CodeTypeDeclaration enumerator = Assert.Single(
+                collection.Members.OfType<CodeTypeDeclaration>(),
+                type => type.Members.OfType<CodeMemberField>().Any(field => field.Name == "privObjEnum"));
+            AssertDisposableWrapper(enumerator, "privObjEnum");
+        }

This test checks that the generated collection and enumerator types both implement IDisposable and that their Dispose methods forward to the correct underlying field.

+        private static void AssertDisposableWrapper(CodeTypeDeclaration wrapper, string wrappedFieldName)
+        {
+            Assert.Contains(wrapper.BaseTypes.Cast<CodeTypeReference>(), type => type.BaseType == typeof(IDisposable).FullName);
+
+            CodeMemberMethod disposeMethod = Assert.Single(
+                wrapper.Members.OfType<CodeMemberMethod>(),
+                method => method.Name == nameof(IDisposable.Dispose));
+            Assert.Contains(
+                disposeMethod.ImplementationTypes.Cast<CodeTypeReference>(),
+                type => type.BaseType == typeof(IDisposable).FullName);
+
+            CodeExpressionStatement statement = Assert.IsType<CodeExpressionStatement>(
+                Assert.Single(disposeMethod.Statements.Cast<CodeStatement>()));
+            CodeMethodInvokeExpression invocation = Assert.IsType<CodeMethodInvokeExpression>(statement.Expression);
+            Assert.Equal(nameof(IDisposable.Dispose), invocation.Method.MethodName);
+            CodeVariableReferenceExpression target = Assert.IsType<CodeVariableReferenceExpression>(invocation.Method.TargetObject);
+            Assert.Equal(wrappedFieldName, target.VariableName);
+        }

This helper method asserts that the generated type implements IDisposable and that its Dispose method correctly delegates to the wrapped field's Dispose.

From the review thread

artl93:
@jeffhandley - is this a behavior breaking change since we're adding to the returned object?

tannergooding:
Is this missing the finalizer and the rest of the standard IDisposable pattern?

-- Noting that may have other impact into the breaking change qualification, but its something to consider since implementing Dispose itself doesn't guarantee the resource is cleaned up. You need the finalizer to cover cases where the user doesn't call Dispose so it is cleaned when the GC collects it instead.

You still have potential issues with derived types, but this is a dynamically generated type and so the risk of there being a derived type is "lesser" (just not non-existent)

bartonjs:
If the type isn't sealed, then it should implement the Basic Dispose Pattern.

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

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        privObjEnum.Dispose();
    }
}

That doesn't include preemptively adding a finalizer, just supporting that a derived type could add one.

Takeaways

• When adding IDisposable to a non-sealed type, always implement the full dispose pattern (Dispose(), Dispose(bool), and GC.SuppressFinalize(this)) to support correct disposal in derived types.
• Exposing disposal for objects that wrap disposable resources enables deterministic resource cleanup and aligns with .NET best practices, but can introduce source and behavioral breaking changes if consumers have derived types or conflicting members.
• Testing code generation changes should include reflection or code DOM checks to ensure the generated types implement the intended interfaces and correctly forward disposal calls.