Skip to content

Commit 2a0cc20

Browse files
weltkanteweltkante
committed
Explicit ImageList ownership management.
1 parent dec24b8 commit 2a0cc20

5 files changed

Lines changed: 175 additions & 37 deletions

File tree

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,20 +61,20 @@ private void Init(IntPtr himl)
6161

6262
public void Dispose()
6363
{
64+
#if DEBUG
65+
GC.SuppressFinalize(this);
66+
#endif
6467
lock (s_syncLock)
6568
{
6669
if (Handle == IntPtr.Zero)
6770
{
6871
return;
6972
}
7073

71-
ComCtl32.ImageList.Destroy(Handle);
74+
var result = ComCtl32.ImageList.Destroy(Handle);
75+
Debug.Assert(result.IsTrue());
7276
Handle = IntPtr.Zero;
7377
}
74-
75-
#if DEBUG
76-
GC.SuppressFinalize(this);
77-
#endif
7878
}
7979

8080
#if DEBUG
@@ -91,6 +91,13 @@ public void Dispose()
9191
}
9292
#endif
9393

94+
internal IntPtr TransferOwnership()
95+
{
96+
var handle = Handle;
97+
Handle = IntPtr.Zero;
98+
return handle;
99+
}
100+
94101
internal NativeImageList Duplicate()
95102
{
96103
lock (s_syncLock)

‎src/System.Windows.Forms/src/System/Windows/Forms/ImageList.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,17 @@ public IntPtr Handle
142142
}
143143
}
144144

145+
internal IntPtr CreateUniqueHandle()
146+
{
147+
if (_nativeImageList == null)
148+
{
149+
CreateHandle();
150+
}
151+
152+
using var iml = _nativeImageList.Duplicate();
153+
return iml.TransferOwnership();
154+
}
155+
145156
/// <summary>
146157
/// Whether or not the underlying Win32 handle has been created.
147158
/// </summary>

‎src/System.Windows.Forms/src/System/Windows/Forms/ListView.cs‎

Lines changed: 140 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -579,11 +579,21 @@ public bool CheckBoxes
579579
{
580580
if (CheckBoxes)
581581
{ // we want custom checkboxes
582-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.Handle);
582+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.CreateUniqueHandle());
583+
if (previousHandle != IntPtr.Zero)
584+
{
585+
var result = ComCtl32.ImageList.Destroy(previousHandle);
586+
Debug.Assert(result.IsTrue());
587+
}
583588
}
584589
else
585590
{
586-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
591+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
592+
if (previousHandle != IntPtr.Zero)
593+
{
594+
var result = ComCtl32.ImageList.Destroy(previousHandle);
595+
Debug.Assert(result.IsTrue());
596+
}
587597
}
588598
}
589599

@@ -664,7 +674,9 @@ protected override CreateParams CreateParams
664674
cp.Style |= (currentStyle & (int)(User32.WS.HSCROLL | User32.WS.VSCROLL));
665675
}
666676

667-
cp.Style |= (int)LVS.SHAREIMAGELISTS;
677+
// disabled until ownership management of list handles is fixed
678+
// https://github.com/dotnet/winforms/issues/3531
679+
//cp.Style |= (int)LVS.SHAREIMAGELISTS;
668680

669681
switch (alignStyle)
670682
{
@@ -966,8 +978,13 @@ public ImageList GroupImageList
966978
return;
967979
}
968980

969-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER,
970-
value is null ? IntPtr.Zero : value.Handle);
981+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER,
982+
value is null ? IntPtr.Zero : value.CreateUniqueHandle());
983+
if (previousHandle != IntPtr.Zero)
984+
{
985+
var result = ComCtl32.ImageList.Destroy(previousHandle);
986+
Debug.Assert(result.IsTrue());
987+
}
971988
}
972989
}
973990

@@ -1235,7 +1252,13 @@ public ImageList LargeImageList
12351252
return;
12361253
}
12371254

1238-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, value is null ? IntPtr.Zero : value.Handle);
1255+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, value is null ? IntPtr.Zero : value.CreateUniqueHandle());
1256+
if (previousHandle != IntPtr.Zero)
1257+
{
1258+
var result = ComCtl32.ImageList.Destroy(previousHandle);
1259+
Debug.Assert(result.IsTrue());
1260+
}
1261+
12391262
if (AutoArrange && !listViewState1[LISTVIEWSTATE1_disposingImageLists])
12401263
{
12411264
UpdateListViewItemsLocations();
@@ -1474,7 +1497,12 @@ public ImageList SmallImageList
14741497
return;
14751498
}
14761499

1477-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, value is null ? IntPtr.Zero : value.Handle);
1500+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, value is null ? IntPtr.Zero : value.CreateUniqueHandle());
1501+
if (previousHandle != IntPtr.Zero)
1502+
{
1503+
var result = ComCtl32.ImageList.Destroy(previousHandle);
1504+
Debug.Assert(result.IsTrue());
1505+
}
14781506

14791507
if (View == View.SmallIcon)
14801508
{
@@ -1582,7 +1610,12 @@ public ImageList StateImageList
15821610

15831611
if (IsHandleCreated)
15841612
{
1585-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, value is null ? IntPtr.Zero : value.Handle);
1613+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, value is null ? IntPtr.Zero : value.CreateUniqueHandle());
1614+
if (previousHandle != IntPtr.Zero)
1615+
{
1616+
var result = ComCtl32.ImageList.Destroy(previousHandle);
1617+
Debug.Assert(result.IsTrue());
1618+
}
15861619
}
15871620
}
15881621
else
@@ -1597,7 +1630,12 @@ public ImageList StateImageList
15971630
// (Yes, it does exactly that even though our wrapper sets LVS_SHAREIMAGELISTS on the native listView.)
15981631
// So we make the native listView forget about its StateImageList just before we recreate the handle.
15991632
// Likely related to https://devblogs.microsoft.com/oldnewthing/20171128-00/?p=97475
1600-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
1633+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
1634+
if (previousHandle != IntPtr.Zero)
1635+
{
1636+
var result = ComCtl32.ImageList.Destroy(previousHandle);
1637+
Debug.Assert(result.IsTrue());
1638+
}
16011639
}
16021640

16031641
_imageListState = value;
@@ -1615,8 +1653,13 @@ public ImageList StateImageList
16151653
}
16161654
else
16171655
{
1618-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE,
1619-
(_imageListState is null || _imageListState.Images.Count == 0) ? IntPtr.Zero : _imageListState.Handle);
1656+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE,
1657+
(_imageListState is null || _imageListState.Images.Count == 0) ? IntPtr.Zero : _imageListState.CreateUniqueHandle());
1658+
if (previousHandle != IntPtr.Zero)
1659+
{
1660+
var result = ComCtl32.ImageList.Destroy(previousHandle);
1661+
Debug.Assert(result.IsTrue());
1662+
}
16201663
}
16211664

16221665
// Comctl should handle auto-arrange for us, but doesn't
@@ -3672,8 +3715,13 @@ private void GroupImageListRecreateHandle(object sender, EventArgs e)
36723715
return;
36733716
}
36743717

3675-
IntPtr handle = (GroupImageList is null) ? IntPtr.Zero : GroupImageList.Handle;
3676-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, handle);
3718+
IntPtr handle = (GroupImageList is null) ? IntPtr.Zero : GroupImageList.CreateUniqueHandle();
3719+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, handle);
3720+
if (previousHandle != IntPtr.Zero)
3721+
{
3722+
var result = ComCtl32.ImageList.Destroy(previousHandle);
3723+
Debug.Assert(result.IsTrue());
3724+
}
36773725
}
36783726

36793727
public ListViewHitTestInfo HitTest(Point point)
@@ -4243,8 +4291,13 @@ private void LargeImageListRecreateHandle(object sender, EventArgs e)
42434291
return;
42444292
}
42454293

4246-
IntPtr handle = (LargeImageList is null) ? IntPtr.Zero : LargeImageList.Handle;
4247-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, handle);
4294+
IntPtr handle = (LargeImageList is null) ? IntPtr.Zero : LargeImageList.CreateUniqueHandle();
4295+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, handle);
4296+
if (previousHandle != IntPtr.Zero)
4297+
{
4298+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4299+
Debug.Assert(result.IsTrue());
4300+
}
42484301

42494302
ForceCheckBoxUpdate();
42504303
}
@@ -4587,6 +4640,34 @@ protected override void OnHandleCreated(EventArgs e)
45874640

45884641
protected override void OnHandleDestroyed(EventArgs e)
45894642
{
4643+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL);
4644+
if (previousHandle != IntPtr.Zero)
4645+
{
4646+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4647+
Debug.Assert(result.IsTrue());
4648+
}
4649+
4650+
previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL);
4651+
if (previousHandle != IntPtr.Zero)
4652+
{
4653+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4654+
Debug.Assert(result.IsTrue());
4655+
}
4656+
4657+
previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE);
4658+
if (previousHandle != IntPtr.Zero)
4659+
{
4660+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4661+
Debug.Assert(result.IsTrue());
4662+
}
4663+
4664+
previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER);
4665+
if (previousHandle != IntPtr.Zero)
4666+
{
4667+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4668+
Debug.Assert(result.IsTrue());
4669+
}
4670+
45904671
// don't save the list view items state when in virtual mode : it is the responsability of the
45914672
// user to cache the list view items in virtual mode
45924673
if (!Disposing && !VirtualMode)
@@ -4822,22 +4903,42 @@ protected void RealizeProperties()
48224903

48234904
if (_imageListLarge != null)
48244905
{
4825-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, _imageListLarge.Handle);
4906+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.NORMAL, _imageListLarge.CreateUniqueHandle());
4907+
if (previousHandle != IntPtr.Zero)
4908+
{
4909+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4910+
Debug.Assert(result.IsTrue());
4911+
}
48264912
}
48274913

48284914
if (_imageListSmall != null)
48294915
{
4830-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, _imageListSmall.Handle);
4916+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, _imageListSmall.CreateUniqueHandle());
4917+
if (previousHandle != IntPtr.Zero)
4918+
{
4919+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4920+
Debug.Assert(result.IsTrue());
4921+
}
48314922
}
48324923

48334924
if (_imageListState != null)
48344925
{
4835-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.Handle);
4926+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, _imageListState.CreateUniqueHandle());
4927+
if (previousHandle != IntPtr.Zero)
4928+
{
4929+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4930+
Debug.Assert(result.IsTrue());
4931+
}
48364932
}
48374933

48384934
if (_imageListGroup != null)
48394935
{
4840-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, _imageListGroup.Handle);
4936+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.GROUPHEADER, _imageListGroup.CreateUniqueHandle());
4937+
if (previousHandle != IntPtr.Zero)
4938+
{
4939+
var result = ComCtl32.ImageList.Destroy(previousHandle);
4940+
Debug.Assert(result.IsTrue());
4941+
}
48414942
}
48424943
}
48434944

@@ -5364,8 +5465,13 @@ private void SmallImageListRecreateHandle(object sender, EventArgs e)
53645465
return;
53655466
}
53665467

5367-
IntPtr handle = (SmallImageList is null) ? IntPtr.Zero : SmallImageList.Handle;
5368-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, handle);
5468+
IntPtr handle = (SmallImageList is null) ? IntPtr.Zero : SmallImageList.CreateUniqueHandle();
5469+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.SMALL, handle);
5470+
if (previousHandle != IntPtr.Zero)
5471+
{
5472+
var result = ComCtl32.ImageList.Destroy(previousHandle);
5473+
Debug.Assert(result.IsTrue());
5474+
}
53695475

53705476
ForceCheckBoxUpdate();
53715477
}
@@ -5396,8 +5502,13 @@ private void StateImageListRecreateHandle(object sender, EventArgs e)
53965502
return;
53975503
}
53985504

5399-
IntPtr handle = (StateImageList is null) ? IntPtr.Zero : StateImageList.Handle;
5400-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, handle);
5505+
IntPtr handle = (StateImageList is null) ? IntPtr.Zero : StateImageList.CreateUniqueHandle();
5506+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, handle);
5507+
if (previousHandle != IntPtr.Zero)
5508+
{
5509+
var result = ComCtl32.ImageList.Destroy(previousHandle);
5510+
Debug.Assert(result.IsTrue());
5511+
}
54015512
}
54025513

54035514
/// <summary>
@@ -6164,7 +6275,12 @@ internal void RecreateHandleInternal()
61646275
// (Yes, it does exactly that even though our wrapper sets LVS_SHAREIMAGELISTS on the native listView.)
61656276
if (IsHandleCreated && StateImageList != null)
61666277
{
6167-
User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
6278+
var previousHandle = User32.SendMessageW(this, (User32.WM)LVM.SETIMAGELIST, (IntPtr)LVSIL.STATE, IntPtr.Zero);
6279+
if (previousHandle != IntPtr.Zero)
6280+
{
6281+
var result = ComCtl32.ImageList.Destroy(previousHandle);
6282+
Debug.Assert(result.IsTrue());
6283+
}
61686284
}
61696285

61706286
RecreateHandle();

0 commit comments

Comments
 (0)