https://git.reactos.org/?p=reactos.git;a=commitdiff;h=da8a41b97bef45f28a1c85c17b99b1bfe22b4008
commit da8a41b97bef45f28a1c85c17b99b1bfe22b4008
Author: Pierre Schweitzer <pie...@reactos.org>
AuthorDate: Tue Mar 6 20:22:50 2018 +0100
Commit: Pierre Schweitzer <pie...@reactos.org>
CommitDate: Tue Mar 6 20:30:21 2018 +0100
[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
---
dll/win32/shell32/wine/changenotify.c | 56 ++++++++---------------------------
1 file changed, 12 insertions(+), 44 deletions(-)
diff --git a/dll/win32/shell32/wine/changenotify.c
b/dll/win32/shell32/wine/changenotify.c
index 0157297e1b..05c7284a87 100644
--- 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 */
- struct _NOTIFICATIONLIST * pParent;
} SHChangeNotifyEntryInternal, *LPNOTIFYREGISTER;
#else
typedef SHChangeNotifyEntry *LPNOTIFYREGISTER;
@@ -76,9 +75,6 @@ typedef struct _NOTIFICATIONLIST
LONG wEventMask; /* subscribed events */
DWORD dwFlags; /* client flags */
ULONG id;
-#ifdef __REACTOS__
- volatile LONG wQueuedCount;
-#endif
} NOTIFICATIONLIST, *LPNOTIFICATIONLIST;
#ifdef __REACTOS__
@@ -161,22 +157,9 @@ static const char * NodeName(const NOTIFICATIONLIST *item)
static void DeleteNode(LPNOTIFICATIONLIST item)
{
UINT i;
-#ifdef __REACTOS__
- LONG queued;
-#endif
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 );
@@ -253,7 +236,6 @@ SHChangeNotifyRegister(
item->cidl = cItems;
#ifdef __REACTOS__
item->apidl = SHAlloc(sizeof(SHChangeNotifyEntryInternal) * cItems);
- item->wQueuedCount = 0;
#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].pParent = item;
if (fSources & SHCNRF_InterruptLevel)
{
if (_OpenDirectory( &item->apidl[i] ))
{
- InterlockedIncrement(&item->wQueuedCount);
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");
- 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. */
@@ -755,11 +745,7 @@ _NotificationCompletion(DWORD dwErrorCode, // completion
code
item->pidl,
NULL);
-#ifdef __REACTOS__
- goto quit;
-#else
return;
-#endif
}
/*
@@ -776,21 +762,12 @@ _NotificationCompletion(DWORD dwErrorCode, // completion
code
_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);
-#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
@@ -800,22 +777,13 @@ static VOID _BeginRead(LPNOTIFYREGISTER item )
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->pParent,
item->hDirectory,
item->buffer,
&item->overlapped,
_NotificationCompletion,
GetLastError());
-#ifdef __REACTOS__
- InterlockedDecrement(&item->pParent->wQueuedCount);
- DeleteNode(item->pParent);
- }
-#endif
}
DWORD _MapAction(DWORD dwAction, BOOL isDir)
