diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.NativeImageList.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.NativeImageList.cs
index 800304c3e41..5ae55105cf8 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.NativeImageList.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.NativeImageList.cs
@@ -61,6 +61,9 @@ private void Init(IntPtr himl)
public void Dispose()
{
+#if DEBUG
+ GC.SuppressFinalize(this);
+#endif
lock (s_syncLock)
{
if (Handle == IntPtr.Zero)
@@ -68,13 +71,10 @@ public void Dispose()
return;
}
- ComCtl32.ImageList.Destroy(Handle);
+ var result = ComCtl32.ImageList.Destroy(Handle);
+ Debug.Assert(result.IsTrue());
Handle = IntPtr.Zero;
}
-
-#if DEBUG
- GC.SuppressFinalize(this);
-#endif
}
#if DEBUG
@@ -91,6 +91,13 @@ public void Dispose()
}
#endif
+ internal IntPtr TransferOwnership()
+ {
+ var handle = Handle;
+ Handle = IntPtr.Zero;
+ return handle;
+ }
+
internal NativeImageList Duplicate()
{
lock (s_syncLock)
diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.cs
index 5670a99d6a8..9e44835c61b 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/ImageList.cs
@@ -142,6 +142,17 @@ public IntPtr Handle
}
}
+ internal IntPtr CreateUniqueHandle()
+ {
+ if (_nativeImageList == null)
+ {
+ CreateHandle();
+ }
+
+ using var iml = _nativeImageList.Duplicate();
+ return iml.TransferOwnership();
+ }
+
///
/// Whether or not the underlying Win32 handle has been created.
///
diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ListView.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ListView.cs
index 2d8353f0741..a9add845ed2 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/ListView.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/ListView.cs
@@ -579,11 +579,21 @@ public bool CheckBoxes
{
if (CheckBoxes)
{ // we want custom checkboxes
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
else
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
}
@@ -664,7 +674,9 @@ protected override CreateParams CreateParams
cp.Style |= (currentStyle & (int)(User32.WS.HSCROLL | User32.WS.VSCROLL));
}
- cp.Style |= (int)LVS.SHAREIMAGELISTS;
+ // disabled until ownership management of list handles is fixed
+ // https://github.com/dotnet/winforms/issues/3531
+ //cp.Style |= (int)LVS.SHAREIMAGELISTS;
switch (alignStyle)
{
@@ -966,8 +978,13 @@ public ImageList GroupImageList
return;
}
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER,
- value is null ? IntPtr.Zero : value.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER,
+ value is null ? IntPtr.Zero : value.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
}
@@ -1235,7 +1252,13 @@ public ImageList LargeImageList
return;
}
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, value is null ? IntPtr.Zero : value.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, value is null ? IntPtr.Zero : value.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
+
if (AutoArrange && !listViewState1[LISTVIEWSTATE1_disposingImageLists])
{
UpdateListViewItemsLocations();
@@ -1474,7 +1497,12 @@ public ImageList SmallImageList
return;
}
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, value is null ? IntPtr.Zero : value.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, value is null ? IntPtr.Zero : value.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
if (View == View.SmallIcon)
{
@@ -1582,7 +1610,12 @@ public ImageList StateImageList
if (IsHandleCreated)
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, value is null ? IntPtr.Zero : value.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, value is null ? IntPtr.Zero : value.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
}
else
@@ -1597,7 +1630,12 @@ public ImageList StateImageList
// (Yes, it does exactly that even though our wrapper sets LVS_SHAREIMAGELISTS on the native listView.)
// So we make the native listView forget about its StateImageList just before we recreate the handle.
// Likely related to https://devblogs.microsoft.com/oldnewthing/20171128-00/?p=97475
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
_imageListState = value;
@@ -1615,8 +1653,13 @@ public ImageList StateImageList
}
else
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE,
- (_imageListState is null || _imageListState.Images.Count == 0) ? IntPtr.Zero : _imageListState.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE,
+ (_imageListState is null || _imageListState.Images.Count == 0) ? IntPtr.Zero : _imageListState.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
// Comctl should handle auto-arrange for us, but doesn't
@@ -3672,8 +3715,13 @@ private void GroupImageListRecreateHandle(object sender, EventArgs e)
return;
}
- IntPtr handle = (GroupImageList is null) ? IntPtr.Zero : GroupImageList.Handle;
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, handle);
+ IntPtr handle = (GroupImageList is null) ? IntPtr.Zero : GroupImageList.CreateUniqueHandle();
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, handle);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
public ListViewHitTestInfo HitTest(Point point)
@@ -4243,8 +4291,13 @@ private void LargeImageListRecreateHandle(object sender, EventArgs e)
return;
}
- IntPtr handle = (LargeImageList is null) ? IntPtr.Zero : LargeImageList.Handle;
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, handle);
+ IntPtr handle = (LargeImageList is null) ? IntPtr.Zero : LargeImageList.CreateUniqueHandle();
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, handle);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
ForceCheckBoxUpdate();
}
@@ -4587,6 +4640,34 @@ protected override void OnHandleCreated(EventArgs e)
protected override void OnHandleDestroyed(EventArgs e)
{
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
+
+ previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
+
+ previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
+
+ previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
+
// don't save the list view items state when in virtual mode : it is the responsability of the
// user to cache the list view items in virtual mode
if (!Disposing && !VirtualMode)
@@ -4822,22 +4903,42 @@ protected void RealizeProperties()
if (_imageListLarge != null)
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, _imageListLarge.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, _imageListLarge.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
if (_imageListSmall != null)
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, _imageListSmall.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, _imageListSmall.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
if (_imageListState != null)
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
if (_imageListGroup != null)
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, _imageListGroup.Handle);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, _imageListGroup.CreateUniqueHandle());
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
}
@@ -5364,8 +5465,13 @@ private void SmallImageListRecreateHandle(object sender, EventArgs e)
return;
}
- IntPtr handle = (SmallImageList is null) ? IntPtr.Zero : SmallImageList.Handle;
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, handle);
+ IntPtr handle = (SmallImageList is null) ? IntPtr.Zero : SmallImageList.CreateUniqueHandle();
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, handle);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
ForceCheckBoxUpdate();
}
@@ -5396,8 +5502,13 @@ private void StateImageListRecreateHandle(object sender, EventArgs e)
return;
}
- IntPtr handle = (StateImageList is null) ? IntPtr.Zero : StateImageList.Handle;
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, handle);
+ IntPtr handle = (StateImageList is null) ? IntPtr.Zero : StateImageList.CreateUniqueHandle();
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, handle);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
///
@@ -6164,7 +6275,12 @@ internal void RecreateHandleInternal()
// (Yes, it does exactly that even though our wrapper sets LVS_SHAREIMAGELISTS on the native listView.)
if (IsHandleCreated && StateImageList != null)
{
- User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
+ var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
+ if (previousHandle != IntPtr.Zero)
+ {
+ var result = ComCtl32.ImageList.Destroy(previousHandle);
+ Debug.Assert(result.IsTrue());
+ }
}
RecreateHandle();
diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/TreeView.cs b/src/System.Windows.Forms/src/System/Windows/Forms/TreeView.cs
index ae8b7aad024..3c2b177ac3a 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/TreeView.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/TreeView.cs
@@ -662,7 +662,7 @@ public ImageList ImageList
value == null ? IntPtr.Zero : value.Handle);
if (StateImageList != null && StateImageList.Images.Count > 0 && internalStateImageList != null)
{
- SetStateImageList(internalStateImageList.Handle);
+ SetStateImageList(internalStateImageList.CreateUniqueHandle());
}
}
UpdateCheckedState(root, true);
@@ -1819,7 +1819,7 @@ private void StateImageListRecreateHandle(object sender, EventArgs e)
IntPtr handle = IntPtr.Zero;
if (internalStateImageList != null)
{
- handle = internalStateImageList.Handle;
+ handle = internalStateImageList.CreateUniqueHandle();
}
SetStateImageList(handle);
}
@@ -1859,7 +1859,7 @@ private void StateImageListChangedHandle(object sender, EventArgs e)
internalStateImageList.ImageSize = (Size)ScaledStateImageSize;
}
- SetStateImageList(internalStateImageList.Handle);
+ SetStateImageList(internalStateImageList.CreateUniqueHandle());
}
}
else //stateImageList == null || stateImageList.Images.Count = 0;
@@ -2050,7 +2050,7 @@ private void UpdateNativeStateImageList()
images[i] = stateImageList.Images[i - 1];
}
newImageList.Images.AddRange(images);
- User32.SendMessageW(this, (User32.WM)TVM.SETIMAGELIST, (IntPtr)TVSIL.STATE, newImageList.Handle);
+ User32.SendMessageW(this, (User32.WM)TVM.SETIMAGELIST, (IntPtr)TVSIL.STATE, newImageList.CreateUniqueHandle());
if (internalStateImageList != null)
{
@@ -2067,7 +2067,8 @@ private void SetStateImageList(IntPtr handle)
IntPtr handleOld = User32.SendMessageW(this, (User32.WM)TVM.SETIMAGELIST, (IntPtr)TVSIL.STATE, handle);
if ((handleOld != IntPtr.Zero) && (handleOld != handle))
{
- ComCtl32.ImageList.Destroy(new HandleRef(this, handleOld));
+ var result = ComCtl32.ImageList.Destroy(new HandleRef(this, handleOld));
+ Debug.Assert(result.IsTrue());
}
}
@@ -2078,7 +2079,8 @@ private void DestroyNativeStateImageList(bool reset)
IntPtr handle = User32.SendMessageW(this, (User32.WM)TVM.GETIMAGELIST, (IntPtr)TVSIL.STATE);
if (handle != IntPtr.Zero)
{
- ComCtl32.ImageList.Destroy(new HandleRef(this, handle));
+ var result = ComCtl32.ImageList.Destroy(new HandleRef(this, handle));
+ Debug.Assert(result.IsTrue());
if (reset)
{
User32.SendMessageW(this, (User32.WM)TVM.SETIMAGELIST, (IntPtr)TVSIL.STATE);
@@ -2619,7 +2621,7 @@ internal override void UpdateStylesCore()
// user's images.
if (internalStateImageList != null)
{
- SetStateImageList(internalStateImageList.Handle);
+ SetStateImageList(internalStateImageList.CreateUniqueHandle());
}
}
}
diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ListViewTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ListViewTests.cs
index 3d655d373e9..c3330b37db4 100644
--- a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ListViewTests.cs
+++ b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ListViewTests.cs
@@ -170,7 +170,9 @@ public void ListView_CreateParams_GetDefault_ReturnsExpected()
Assert.Equal(97, createParams.Height);
Assert.Equal(IntPtr.Zero, createParams.Parent);
Assert.Null(createParams.Param);
- Assert.Equal(0x56010148, createParams.Style);
+ // LVS.SHAREIMAGELISTS is temporarily removed from style until ownership management is fixed
+ // https://github.com/dotnet/winforms/issues/3531
+ Assert.Equal(0x56010108, createParams.Style);
Assert.Equal(121, createParams.Width);
Assert.Equal(0, createParams.X);
Assert.Equal(0, createParams.Y);