`NativeImageList` not disposed by `ResourceManager`

[RussKie]

• .NET Version:
  Debug builds of .NET 5.0 and 6.0

• Have you experienced this same bug with .NET Framework?:
  No

Problem description:

We have added the code to track lifetime and incorrect disposal of unmanaged resources, specifically in NativeImageList we have the following code:

winforms/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.NativeImageList.cs

Lines 79 to 91 in b30bdd1

	#if DEBUG
	private readonly string _callStack = new StackTrace().ToString();
	~NativeImageList()
	{
	Debug.Fail($"{nameof(NativeImageList)} was not disposed properly. Originating stack:\n{_callStack}");
	// We can't do anything with the fields when we're on the finalizer as they're all classes. If any of
	// them become structs they'll be a part of this instance and possible to clean up. Ideally we fix
	// the leaks and never come in on the finalizer.
	return;
	}
	#endif

This code causes apps to crash if those apps have serialised imagelists, and have a code similar to the following:

this.imageList2.ImageStream = (System.Windows.Forms.ImageListStreamer)(resources.GetObject("imageList2.ImageStream"));

The app will crash with the following stacktrace:

Message: NativeImageList was not disposed properly. Originating stack:
   at System.Windows.Forms.ImageList.NativeImageList..ctor(IStream pstm)
   at System.Windows.Forms.ImageListStreamer..ctor(SerializationInfo info, StreamingContext context)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeConstructorInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at System.Runtime.Serialization.ObjectManager.CompleteISerializableObject(Object obj, SerializationInfo info, StreamingContext context)
   at System.Runtime.Serialization.ObjectManager.FixupSpecialObject(ObjectHolder holder)
   at System.Runtime.Serialization.ObjectManager.DoFixups()
   at System.Runtime.Serialization.Formatters.Binary.ObjectReader.Deserialize(BinaryParser serParser)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize(Stream serializationStream)
   at System.Resources.Extensions.DeserializingResourceReader.ReadBinaryFormattedObject()
   at System.Resources.Extensions.DeserializingResourceReader.DeserializeObject(Int32 typeIndex)
   at System.Resources.Extensions.DeserializingResourceReader._LoadObjectV2(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.DeserializingResourceReader.LoadObjectV2(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.DeserializingResourceReader.LoadObject(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString)
   at System.Resources.Extensions.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase)
   at System.Resources.ResourceManager.GetObject(String name, CultureInfo culture, Boolean wrapUnmanagedMemStream)
   at System.Resources.ResourceManager.GetObject(String name)
   at SimpleWinForms.Form1.InitializeComponent()
   at SimpleWinForms.Form1..ctor()
   at SimpleWinForms.Program.Main()

Expected behavior:

All objects are disposed correctly, and the assert is not hit.

Minimal repro:

1. Build the Windows Forms solution in Debug mode
2. Run WinformsControlsTest project from Visual Studio with a debugger attached
3. Open ListView form
4. Observe VS hit the assert

[weltkante]

We have discussed this before in #3567 so this is probably a duplicate, ResourceManager is not responsible for disposing it, its a factory and its lifetime is detached from the objects it produces.

In my opinion the ownership of the ImageListStreamer should be passed on to the ImageList control. If thats not possible (because it breaks compatibility) then you may have to revert the strict asserts and have NativeImageList be released in a finalizer. Or maybe add special support code in ImageListStreamer finalizer.

Notice this is only a debug fail, release builds should not hit this I think? (but leak the ImageList of course, which is #3567) Unit tests do take precaution to Dispose the ImageListStreamer objects I believe.

[RussKie]

Man you're good! I've completely forgotten about this one.
Thank you for pointing it out.

[Amy-Li03]

This issue has been fixed on .NET SDK 5.0.8 servicing, we can open ListView form successfully and no VS crash.