[comctl32] Fix SEGV in file dialogs (bug 3366)

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[comctl32] Fix SEGV in file dialogs (bug 3366)

Troy Rollo
The following change seems to prevent the SEGV that frequently occurs when you
double-click on a folder in the file open/save dialogs. This fix uses
IsWindow() to test if the ListView still exists. I have handled all
notifications other than those where it seems entirely implausible that the
callback could destroy the window.

ChangeLog:
        Detect when the notification callback has destroyed the ListView to avoid
        attempts to access data that is no longer valid.

=== dlls/comctl32/listview.c
==================================================================
--- dlls/comctl32/listview.c (revision 43320)
+++ dlls/comctl32/listview.c (local)
@@ -3018,6 +3018,7 @@
 {
     INT nFirst = min(infoPtr->nSelectionMark, nItem);
     INT nLast = max(infoPtr->nSelectionMark, nItem);
+    HWND hwndSelf = infoPtr->hwndSelf;
     NMLVODSTATECHANGE nmlv;
     LVITEMW item;
     BOOL bOldChange;
@@ -3046,6 +3047,8 @@
     nmlv.uOldState = item.state;
 
     notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv);
+    if (!IsWindow(hwndSelf))
+ return;
     infoPtr->bDoChangeNotify = bOldChange;
 }
 
@@ -3400,9 +3403,15 @@
     /* send LVN_ITEMCHANGING notification, if the item is not being inserted */
     /* and we are _NOT_ virtual (LVS_OWERNDATA), and change notifications */
     /* are enabled */
-    if(lpItem && !isNew && infoPtr->bDoChangeNotify &&
-       notify_listview(infoPtr, LVN_ITEMCHANGING, &nmlv))
+    if(lpItem && !isNew && infoPtr->bDoChangeNotify)
+    {
+      HWND hwndSelf = infoPtr->hwndSelf;
+
+      if (notify_listview(infoPtr, LVN_ITEMCHANGING, &nmlv))
  return FALSE;
+      if (!IsWindow(hwndSelf))
+ return FALSE;
+    }
 
     /* copy information */
     if (lpLVItem->mask & LVIF_TEXT)
@@ -3546,6 +3555,7 @@
 static BOOL LISTVIEW_SetItemT(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem, BOOL isW)
 {
     UINT uView = infoPtr->dwStyle & LVS_TYPEMASK;
+    HWND hwndSelf = infoPtr->hwndSelf;
     LPWSTR pszText = NULL;
     BOOL bResult, bChanged = FALSE;
     
@@ -3568,6 +3578,8 @@
  bResult = set_sub_item(infoPtr, lpLVItem, TRUE, &bChanged);
     else
  bResult = set_main_item(infoPtr, lpLVItem, FALSE, TRUE, &bChanged);
+    if (!IsWindow(hwndSelf))
+ return FALSE;
 
     /* redraw item, if necessary */
     if (bChanged && !infoPtr->bIsDrawing)
@@ -4504,6 +4516,7 @@
 static BOOL LISTVIEW_DeleteItem(LISTVIEW_INFO *infoPtr, INT nItem)
 {
     UINT uView = infoPtr->dwStyle & LVS_TYPEMASK;
+    HWND hwndSelf = infoPtr->hwndSelf;
     LVITEMW item;
 
     TRACE("(nItem=%d)\n", nItem);
@@ -4517,6 +4530,8 @@
     
     /* send LVN_DELETEITEM notification. */
     notify_deleteitem(infoPtr, nItem);
+    if (!IsWindow(hwndSelf))
+ return FALSE;
 
     /* we need to do this here, because we'll be deleting stuff */  
     if (uView == LVS_SMALLICON || uView == LVS_ICON)
@@ -4568,6 +4583,7 @@
  */
 static BOOL LISTVIEW_EndEditLabelT(LISTVIEW_INFO *infoPtr, LPWSTR pszText, BOOL isW)
 {
+    HWND hwndSelf = infoPtr->hwndSelf;
     NMLVDISPINFOW dispInfo;
 
     TRACE("(pszText=%s, isW=%d)\n", debugtext_t(pszText, isW), isW);
@@ -4585,6 +4601,8 @@
 
     /* Do we need to update the Item Text */
     if (!notify_dispinfoT(infoPtr, LVN_ENDLABELEDITW, &dispInfo, isW)) return FALSE;
+    if (!IsWindow(hwndSelf))
+ return FALSE;
     if (!pszText) return TRUE;
 
     if (!(infoPtr->dwStyle & LVS_OWNERDATA))
@@ -4625,6 +4643,7 @@
     WCHAR szDispText[DISP_TEXT_SIZE] = { 0 };
     NMLVDISPINFOW dispInfo;
     RECT rect;
+    HWND hwndSelf = infoPtr->hwndSelf;
 
     TRACE("(nItem=%d, isW=%d)\n", nItem, isW);
 
@@ -4662,6 +4681,8 @@
     
     if (notify_dispinfoT(infoPtr, LVN_BEGINLABELEDITW, &dispInfo, isW))
     {
+ if (!IsWindow(hwndSelf))
+    return 0;
  SendMessageW(infoPtr->hwndEdit, WM_CLOSE, 0, 0);
  infoPtr->hwndEdit = 0;
  return 0;
@@ -6191,6 +6212,7 @@
     ITEM_INFO *lpItem;
     BOOL is_sorted, has_changed;
     LVITEMW item;
+    HWND hwndSelf = infoPtr->hwndSelf;
 
     TRACE("(lpLVItem=%s, isW=%d)\n", debuglvitem_t(lpLVItem, isW), isW);
 
@@ -6264,6 +6286,8 @@
     nmlv.iItem = nItem;
     nmlv.lParam = lpItem->lParam;
     notify_listview(infoPtr, LVN_INSERTITEM, &nmlv);
+    if (!IsWindow(hwndSelf))
+ return -1;
 
     /* align items (set position of each item) */
     if ((uView == LVS_SMALLICON || uView == LVS_ICON))
@@ -7967,6 +7991,7 @@
 static LRESULT LISTVIEW_KeyDown(LISTVIEW_INFO *infoPtr, INT nVirtualKey, LONG lKeyData)
 {
   UINT uView =  infoPtr->dwStyle & LVS_TYPEMASK;
+  HWND hwndSelf = infoPtr->hwndSelf;
   INT nItem = -1;
   NMLVKEYDOWN nmKeyDown;
 
@@ -7976,6 +8001,8 @@
   nmKeyDown.wVKey = nVirtualKey;
   nmKeyDown.flags = 0;
   notify_hdr(infoPtr, LVN_KEYDOWN, &nmKeyDown.hdr);
+  if (!IsWindow(hwndSelf))
+    return 0;
 
   switch (nVirtualKey)
   {
@@ -7983,7 +8010,11 @@
     if ((infoPtr->nItemCount > 0) && (infoPtr->nFocusedItem != -1))
     {
       notify(infoPtr, NM_RETURN);
+      if (!IsWindow(hwndSelf))
+ return 0;
       notify(infoPtr, LVN_ITEMACTIVATE);
+      if (!IsWindow(hwndSelf))
+ return 0;
     }
     break;
 
@@ -8063,6 +8094,7 @@
  */
 static LRESULT LISTVIEW_KillFocus(LISTVIEW_INFO *infoPtr)
 {
+    HWND hwndSelf = infoPtr->hwndSelf;
     TRACE("()\n");
 
     /* if we did not have the focus, there's nothing to do */
@@ -8070,6 +8102,8 @@
   
     /* send NM_KILLFOCUS notification */
     notify(infoPtr, NM_KILLFOCUS);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     /* if we have a focus rectagle, get rid of it */
     LISTVIEW_ShowFocusRect(infoPtr, FALSE);
@@ -8098,11 +8132,14 @@
 static LRESULT LISTVIEW_LButtonDblClk(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, INT y)
 {
     LVHITTESTINFO htInfo;
+    HWND hwndSelf = infoPtr->hwndSelf;
 
     TRACE("(key=%hu, X=%hu, Y=%hu)\n", wKey, x, y);
 
     /* send NM_RELEASEDCAPTURE notification */
     notify(infoPtr, NM_RELEASEDCAPTURE);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     htInfo.pt.x = x;
     htInfo.pt.y = y;
@@ -8110,6 +8147,8 @@
     /* send NM_DBLCLK notification */
     LISTVIEW_HitTest(infoPtr, &htInfo, TRUE, FALSE);
     notify_click(infoPtr, NM_DBLCLK, &htInfo);
+    if (!IsWindow(hwndSelf))
+    return 0;
 
     /* To send the LVN_ITEMACTIVATE, it must be on an Item */
     if(htInfo.iItem != -1) notify_itemactivate(infoPtr,&htInfo);
@@ -8134,12 +8173,15 @@
   LVHITTESTINFO lvHitTestInfo;
   static BOOL bGroupSelect = TRUE;
   POINT pt = { x, y };
+  HWND hwndSelf = infoPtr->hwndSelf;
   INT nItem;
 
   TRACE("(key=%hu, X=%hu, Y=%hu)\n", wKey, x, y);
 
   /* send NM_RELEASEDCAPTURE notification */
   notify(infoPtr, NM_RELEASEDCAPTURE);
+  if (!IsWindow(hwndSelf))
+  return FALSE;
 
   if (!infoPtr->bFocus) SetFocus(infoPtr->hwndSelf);
 
@@ -8183,6 +8225,8 @@
         if (bGroupSelect)
  {
           LISTVIEW_AddGroupSelection(infoPtr, nItem);
+  if (!IsWindow(hwndSelf))
+    return 0;
       LISTVIEW_SetItemFocus(infoPtr, nItem);
           infoPtr->nSelectionMark = nItem;
  }
@@ -8247,6 +8291,7 @@
 static LRESULT LISTVIEW_LButtonUp(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, INT y)
 {
     LVHITTESTINFO lvHitTestInfo;
+    HWND hwndSelf = infoPtr->hwndSelf;
     
     TRACE("(key=%hu, X=%hu, Y=%hu)\n", wKey, x, y);
 
@@ -8258,6 +8303,8 @@
     /* send NM_CLICK notification */
     LISTVIEW_HitTest(infoPtr, &lvHitTestInfo, TRUE, FALSE);
     notify_click(infoPtr, NM_CLICK, &lvHitTestInfo);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     /* set left button flag */
     infoPtr->bLButtonDown = FALSE;
@@ -8332,6 +8379,7 @@
 static LRESULT LISTVIEW_HeaderNotification(LISTVIEW_INFO *infoPtr, const NMHEADERW *lpnmh)
 {
     UINT uView =  infoPtr->dwStyle & LVS_TYPEMASK;
+    HWND hwndSelf = infoPtr->hwndSelf;
     
     TRACE("(lpnmh=%p)\n", lpnmh);
 
@@ -8345,6 +8393,8 @@
  case HDN_ITEMCHANGEDW:
  case HDN_ITEMCHANGEDA:
             notify_forward_header(infoPtr, lpnmh);
+    if (!IsWindow(hwndSelf))
+ break;
             /* Fall through */
  case HDN_TRACKW:
  case HDN_TRACKA:
@@ -8571,11 +8621,14 @@
 static LRESULT LISTVIEW_RButtonDblClk(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, INT y)
 {
     LVHITTESTINFO lvHitTestInfo;
+    HWND hwndSelf = infoPtr->hwndSelf;
     
     TRACE("(key=%hu,X=%hu,Y=%hu)\n", wKey, x, y);
 
     /* send NM_RELEASEDCAPTURE notification */
     notify(infoPtr, NM_RELEASEDCAPTURE);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     /* send NM_RDBLCLK notification */
     lvHitTestInfo.pt.x = x;
@@ -8602,11 +8655,14 @@
 {
     LVHITTESTINFO lvHitTestInfo;
     INT nItem;
+    HWND hwndSelf = infoPtr->hwndSelf;
 
     TRACE("(key=%hu,X=%hu,Y=%hu)\n", wKey, x, y);
 
     /* send NM_RELEASEDCAPTURE notification */
     notify(infoPtr, NM_RELEASEDCAPTURE);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     /* make sure the listview control window has the focus */
     if (!infoPtr->bFocus) SetFocus(infoPtr->hwndSelf);
@@ -8649,6 +8705,7 @@
 static LRESULT LISTVIEW_RButtonUp(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, INT y)
 {
     LVHITTESTINFO lvHitTestInfo;
+    HWND hwndSelf = infoPtr->hwndSelf;
     POINT pt;
 
     TRACE("(key=%hu,X=%hu,Y=%hu)\n", wKey, x, y);
@@ -8663,6 +8720,8 @@
     lvHitTestInfo.pt.y = y;
     LISTVIEW_HitTest(infoPtr, &lvHitTestInfo, TRUE, FALSE);
     notify_click(infoPtr, NM_RCLICK, &lvHitTestInfo);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     /* Change to screen coordinate for WM_CONTEXTMENU */
     pt = lvHitTestInfo.pt;
@@ -8719,6 +8778,8 @@
  */
 static LRESULT LISTVIEW_SetFocus(LISTVIEW_INFO *infoPtr, HWND hwndLoseFocus)
 {
+    HWND hwndSelf = infoPtr->hwndSelf;
+
     TRACE("(hwndLoseFocus=%p)\n", hwndLoseFocus);
 
     /* if we have the focus already, there's nothing to do */
@@ -8726,6 +8787,8 @@
   
     /* send NM_SETFOCUS notification */
     notify(infoPtr, NM_SETFOCUS);
+    if (!IsWindow(hwndSelf))
+ return 0;
 
     /* set window focus flag */
     infoPtr->bFocus = TRUE;