Skip to content

Commit f4b2e53

Browse files
HeisSpiterJoachimHenze
authored andcommitted
[0.4.8] cherry-pick [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 (cherry picked from commit da8a41b) cherry-picked by Joachim Henze with many thanks to Pierre.
1 parent 5231a7f commit f4b2e53

File tree

1 file changed

+12
-44
lines changed

1 file changed

+12
-44
lines changed

dll/win32/shell32/wine/changenotify.c

Lines changed: 12 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ typedef struct {
5959
OVERLAPPED overlapped; /* Overlapped structure */
6060
BYTE *buffer; /* Async buffer to fill */
6161
BYTE *backBuffer; /* Back buffer to swap buffer into */
62-
struct _NOTIFICATIONLIST * pParent;
6362
} SHChangeNotifyEntryInternal, *LPNOTIFYREGISTER;
6463
#else
6564
typedef SHChangeNotifyEntry *LPNOTIFYREGISTER;
@@ -76,9 +75,6 @@ typedef struct _NOTIFICATIONLIST
7675
LONG wEventMask; /* subscribed events */
7776
DWORD dwFlags; /* client flags */
7877
ULONG id;
79-
#ifdef __REACTOS__
80-
volatile LONG wQueuedCount;
81-
#endif
8278
} NOTIFICATIONLIST, *LPNOTIFICATIONLIST;
8379

8480
#ifdef __REACTOS__
@@ -161,22 +157,9 @@ static const char * NodeName(const NOTIFICATIONLIST *item)
161157
static void DeleteNode(LPNOTIFICATIONLIST item)
162158
{
163159
UINT i;
164-
#ifdef __REACTOS__
165-
LONG queued;
166-
#endif
167160

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

170-
#ifdef __REACTOS__
171-
queued = InterlockedCompareExchange(&item->wQueuedCount, 0, 0);
172-
if (queued != 0)
173-
{
174-
ERR("Not freeing, still %d queued events\n", queued);
175-
return;
176-
}
177-
TRACE("Freeing for real! %p (%d) \n", item, item->cidl);
178-
#endif
179-
180163
/* remove item from list */
181164
list_remove( &item->entry );
182165

@@ -253,7 +236,6 @@ SHChangeNotifyRegister(
253236
item->cidl = cItems;
254237
#ifdef __REACTOS__
255238
item->apidl = SHAlloc(sizeof(SHChangeNotifyEntryInternal) * cItems);
256-
item->wQueuedCount = 0;
257239
#else
258240
item->apidl = SHAlloc(sizeof(SHChangeNotifyEntry) * cItems);
259241
#endif
@@ -268,13 +250,11 @@ SHChangeNotifyRegister(
268250
item->apidl[i].buffer = SHAlloc(BUFFER_SIZE);
269251
item->apidl[i].backBuffer = SHAlloc(BUFFER_SIZE);
270252
item->apidl[i].overlapped.hEvent = &item->apidl[i];
271-
item->apidl[i].pParent = item;
272253

273254
if (fSources & SHCNRF_InterruptLevel)
274255
{
275256
if (_OpenDirectory( &item->apidl[i] ))
276257
{
277-
InterlockedIncrement(&item->wQueuedCount);
278258
QueueUserAPC( _AddDirectoryProc, m_hThread, (ULONG_PTR) &item->apidl[i] );
279259
}
280260
}
@@ -738,8 +718,18 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
738718
if (dwErrorCode == ERROR_INVALID_FUNCTION)
739719
{
740720
WARN("Directory watching not supported\n");
741-
goto quit;
721+
return;
722+
}
723+
724+
/* Also, if the notify operation was canceled (like, user moved to another
725+
* directory), then, don't requeue notification
726+
*/
727+
if (dwErrorCode == ERROR_OPERATION_ABORTED)
728+
{
729+
TRACE("Notification aborted\n");
730+
return;
742731
}
732+
743733
#endif
744734

745735
/* This likely means overflow, so force whole directory refresh. */
@@ -755,11 +745,7 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
755745
item->pidl,
756746
NULL);
757747

758-
#ifdef __REACTOS__
759-
goto quit;
760-
#else
761748
return;
762-
#endif
763749
}
764750

765751
/*
@@ -776,21 +762,12 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
776762
_BeginRead(item);
777763

778764
_ProcessNotification(item);
779-
780-
#ifdef __REACTOS__
781-
quit:
782-
InterlockedDecrement(&item->pParent->wQueuedCount);
783-
DeleteNode(item->pParent);
784-
#endif
785765
}
786766

787767
static VOID _BeginRead(LPNOTIFYREGISTER item )
788768
{
789769
TRACE("_BeginRead %p \n", item->hDirectory);
790770

791-
#ifdef __REACTOS__
792-
InterlockedIncrement(&item->pParent->wQueuedCount);
793-
#endif
794771
/* This call needs to be reissued after every APC. */
795772
if (!ReadDirectoryChangesW(item->hDirectory, // handle to directory
796773
item->buffer, // read results buffer
@@ -800,22 +777,13 @@ static VOID _BeginRead(LPNOTIFYREGISTER item )
800777
NULL, // bytes returned
801778
&item->overlapped, // overlapped buffer
802779
_NotificationCompletion)) // completion routine
803-
#ifdef __REACTOS__
804-
{
805-
#endif
806-
ERR("ReadDirectoryChangesW failed. (%p, %p, %p, %p, %p, %p) Code: %u \n",
780+
ERR("ReadDirectoryChangesW failed. (%p, %p, %p, %p, %p) Code: %u \n",
807781
item,
808-
item->pParent,
809782
item->hDirectory,
810783
item->buffer,
811784
&item->overlapped,
812785
_NotificationCompletion,
813786
GetLastError());
814-
#ifdef __REACTOS__
815-
InterlockedDecrement(&item->pParent->wQueuedCount);
816-
DeleteNode(item->pParent);
817-
}
818-
#endif
819787
}
820788

821789
DWORD _MapAction(DWORD dwAction, BOOL isDir)

0 commit comments

Comments
 (0)