[SHELL32] Fix a directory handle leak when browsing folders

A bit of history: in r71528, I tried to fix our explorer often
crashing while browsing directories. It was linked to the fact
that a notification result may arrive while the notification
structure had already been deleted.

The fix for this was actually broken and was leading to a double
leak: the notification structure was leaked. But also the handle
to the directory that had been browsed!
This means that the directory couldn't be modified anymore as
a leaked handle to it was still open.

Actually, when notifications are cancel, the kernel properly
calls the notification routine, but with a specific error code.
So the correct fix is to stop handling that notification when
we receive this error code. This is the correct fix with no leaks.

This commit is a complete r71528 revert with the appropriate fix.

CORE-10941
CORE-12843

diff --git a/dll/win32/shell32/wine/changenotify.c b/dll/win32/shell32/wine/changenotify.c
index 0157297..05c7284 100644 (file)
--- a/dll/win32/shell32/wine/changenotify.c
+++ b/dll/win32/shell32/wine/changenotify.c
@@ -59,7 +59,6 @@ typedef struct {
     OVERLAPPED overlapped; /* Overlapped structure */
     BYTE *buffer; /* Async buffer to fill */
     BYTE *backBuffer; /* Back buffer to swap buffer into */
     OVERLAPPED overlapped; /* Overlapped structure */
     BYTE *buffer; /* Async buffer to fill */
     BYTE *backBuffer; /* Back buffer to swap buffer into */

-    struct _NOTIFICATIONLIST * pParent;

 } SHChangeNotifyEntryInternal, *LPNOTIFYREGISTER;
 #else
 typedef SHChangeNotifyEntry *LPNOTIFYREGISTER;
 } SHChangeNotifyEntryInternal, *LPNOTIFYREGISTER;
 #else
 typedef SHChangeNotifyEntry *LPNOTIFYREGISTER;


@@ -76,9 +75,6 @@ typedef struct _NOTIFICATIONLIST
        LONG wEventMask;        /* subscribed events */
        DWORD dwFlags;          /* client flags */
        ULONG id;
        LONG wEventMask;        /* subscribed events */
        DWORD dwFlags;          /* client flags */
        ULONG id;

-#ifdef __REACTOS__
-    volatile LONG wQueuedCount;
-#endif

 } NOTIFICATIONLIST, *LPNOTIFICATIONLIST;
 
 #ifdef __REACTOS__
 } NOTIFICATIONLIST, *LPNOTIFICATIONLIST;
 
 #ifdef __REACTOS__


@@ -161,22 +157,9 @@ static const char * NodeName(const NOTIFICATIONLIST *item)
 static void DeleteNode(LPNOTIFICATIONLIST item)
 {
     UINT i;
 static void DeleteNode(LPNOTIFICATIONLIST item)
 {
     UINT i;

-#ifdef __REACTOS__
-    LONG queued;
-#endif

 
     TRACE("item=%p\n", item);
 
 
     TRACE("item=%p\n", item);
 

-#ifdef __REACTOS__
-    queued = InterlockedCompareExchange(&item->wQueuedCount, 0, 0);
-    if (queued != 0)
-    {
-        ERR("Not freeing, still %d queued events\n", queued);
-        return;
-    }
-    TRACE("Freeing for real! %p (%d) \n", item, item->cidl);
-#endif
-

     /* remove item from list */
     list_remove( &item->entry );
 
     /* remove item from list */
     list_remove( &item->entry );
 


@@ -253,7 +236,6 @@ SHChangeNotifyRegister(
     item->cidl = cItems;
 #ifdef __REACTOS__
     item->apidl = SHAlloc(sizeof(SHChangeNotifyEntryInternal) * cItems);
     item->cidl = cItems;
 #ifdef __REACTOS__
     item->apidl = SHAlloc(sizeof(SHChangeNotifyEntryInternal) * cItems);

-    item->wQueuedCount = 0;

 #else
     item->apidl = SHAlloc(sizeof(SHChangeNotifyEntry) * cItems);
 #endif
 #else
     item->apidl = SHAlloc(sizeof(SHChangeNotifyEntry) * cItems);
 #endif


@@ -268,13 +250,11 @@ SHChangeNotifyRegister(
         item->apidl[i].buffer = SHAlloc(BUFFER_SIZE);
         item->apidl[i].backBuffer = SHAlloc(BUFFER_SIZE);
         item->apidl[i].overlapped.hEvent = &item->apidl[i];
         item->apidl[i].buffer = SHAlloc(BUFFER_SIZE);
         item->apidl[i].backBuffer = SHAlloc(BUFFER_SIZE);
         item->apidl[i].overlapped.hEvent = &item->apidl[i];

-        item->apidl[i].pParent = item;

 
         if (fSources & SHCNRF_InterruptLevel)
         {
             if (_OpenDirectory( &item->apidl[i] ))
             {
 
         if (fSources & SHCNRF_InterruptLevel)
         {
             if (_OpenDirectory( &item->apidl[i] ))
             {

-                InterlockedIncrement(&item->wQueuedCount);

                 QueueUserAPC( _AddDirectoryProc, m_hThread, (ULONG_PTR) &item->apidl[i] );
             }
         }
                 QueueUserAPC( _AddDirectoryProc, m_hThread, (ULONG_PTR) &item->apidl[i] );
             }
         }


@@ -738,8 +718,18 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
     if (dwErrorCode == ERROR_INVALID_FUNCTION)
     {
         WARN("Directory watching not supported\n");
     if (dwErrorCode == ERROR_INVALID_FUNCTION)
     {
         WARN("Directory watching not supported\n");

-        goto quit;
+        return;
+    }
+
+    /* Also, if the notify operation was canceled (like, user moved to another
+     * directory), then, don't requeue notification
+     */
+    if (dwErrorCode == ERROR_OPERATION_ABORTED)
+    {
+        TRACE("Notification aborted\n");
+        return;

     }
     }

+

 #endif
 
     /* This likely means overflow, so force whole directory refresh. */
 #endif
 
     /* This likely means overflow, so force whole directory refresh. */


@@ -755,11 +745,7 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
                        item->pidl,
                        NULL);
 
                        item->pidl,
                        NULL);
 

-#ifdef __REACTOS__
-        goto quit;
-#else

         return;
         return;

-#endif

     }
 
     /*
     }
 
     /*


@@ -776,21 +762,12 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
     _BeginRead(item);
 
     _ProcessNotification(item);
     _BeginRead(item);
 
     _ProcessNotification(item);

-
-#ifdef __REACTOS__
-quit:
-    InterlockedDecrement(&item->pParent->wQueuedCount);
-    DeleteNode(item->pParent);
-#endif

 }
 
 static VOID _BeginRead(LPNOTIFYREGISTER item )
 {
     TRACE("_BeginRead %p \n", item->hDirectory);
 
 }
 
 static VOID _BeginRead(LPNOTIFYREGISTER item )
 {
     TRACE("_BeginRead %p \n", item->hDirectory);
 

-#ifdef __REACTOS__
-    InterlockedIncrement(&item->pParent->wQueuedCount);
-#endif

     /* This call needs to be reissued after every APC. */
     if (!ReadDirectoryChangesW(item->hDirectory, // handle to directory
                                item->buffer, // read results buffer
     /* This call needs to be reissued after every APC. */
     if (!ReadDirectoryChangesW(item->hDirectory, // handle to directory
                                item->buffer, // read results buffer


@@ -800,22 +777,13 @@ static VOID _BeginRead(LPNOTIFYREGISTER item )
                                NULL, // bytes returned
                                &item->overlapped, // overlapped buffer
                                _NotificationCompletion)) // completion routine
                                NULL, // bytes returned
                                &item->overlapped, // overlapped buffer
                                _NotificationCompletion)) // completion routine

-#ifdef __REACTOS__
-    {
-#endif
-        ERR("ReadDirectoryChangesW failed. (%p, %p, %p, %p, %p, %p) Code: %u \n",
+        ERR("ReadDirectoryChangesW failed. (%p, %p, %p, %p, %p) Code: %u \n",

             item,
             item,

-            item->pParent,

             item->hDirectory,
             item->buffer,
             &item->overlapped,
             _NotificationCompletion,
             GetLastError());
             item->hDirectory,
             item->buffer,
             &item->overlapped,
             _NotificationCompletion,
             GetLastError());

-#ifdef __REACTOS__
-        InterlockedDecrement(&item->pParent->wQueuedCount);
-        DeleteNode(item->pParent);
-    }
-#endif

 }
 
 DWORD _MapAction(DWORD dwAction, BOOL isDir)
 }
 
 DWORD _MapAction(DWORD dwAction, BOOL isDir)