[PATCH 4/5] shell32: Implement SHCNRF_InterruptLevel shell notifications.

Nigel Baillie metreckk at gmail.com
Tue Feb 26 18:19:25 CST 2019


When SHChangeNotifyRegister is called with the SHCNRF_InterruptLevel
flag, the caller should receive notifications for external changes to
the filesystem. This is accomplished using ReadDirectoryChangesW, which
is called in a separate thread so that the listener doesn't need to
enter an alertable wait state to receive notifications.

This fixes an issue with new folder/rename operations appearing to not
function from certain paths.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=30752
Signed-off-by: Nigel Baillie <metreckk at gmail.com>
---
 dlls/shell32/changenotify.c | 390 +++++++++++++++++++++++++++++++++++-
 1 file changed, 388 insertions(+), 2 deletions(-)

diff --git a/dlls/shell32/changenotify.c b/dlls/shell32/changenotify.c
index 7656f43fee..bb51ce3b3d 100644
--- a/dlls/shell32/changenotify.c
+++ b/dlls/shell32/changenotify.c
@@ -26,6 +26,7 @@
 #include "wine/list.h"
 #include "wine/debug.h"
 #include "shell32_main.h"
+#include "shlwapi.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(shell);
 
@@ -40,14 +41,35 @@ static CRITICAL_SECTION SHELL32_ChangenotifyCS = { &critsect_debug, -1, 0, 0, 0,
 
 typedef SHChangeNotifyEntry *LPNOTIFYREGISTER;
 
+/* forward declaration */
+struct _NOTIFICATIONLIST;
+
+/* info needed for ReadDirectoryChangesW (internal) */
+typedef struct _DIRCHANGEINFO
+{
+	OVERLAPPED overlapped;
+	struct _NOTIFICATIONLIST *nl;	/* parent NOTIFICATIONLIST */
+	LPNOTIFYREGISTER watch;		/* apidl entry */
+	HANDLE handle;			/* handle being watched */
+	WCHAR listen_path[MAX_PATH];	/* directory being watched */
+	WCHAR *target_file;		/* file being watched (relative to listen_path) */
+	BYTE buffer1[MAX_PATH * 4];	/* buffer to store incoming FILE_NOTIFY_INFORMATION */
+	BYTE buffer2[MAX_PATH * 4];	/* second buffer to avoid race condition */
+	BOOLEAN use_buffer1;		/* TRUE when buffer1 is in use, FALSE when buffer2 is in use */
+} DIRCHANGEINFO;
+
 /* internal list of notification clients (internal) */
 typedef struct _NOTIFICATIONLIST
 {
 	struct list entry;
 	HWND hwnd;		/* window to notify */
 	DWORD uMsg;		/* message to send */
+	DIRCHANGEINFO *changeinfo; /* array of directory change info */
+	HANDLE listener_thread;	/* thread that waits for ReadDirectoryChangesW */
+	HANDLE listener_ready;	/* set when listener is done constructing */
+	HANDLE listener_done;	/* set when listener thread should terminate */
 	LPNOTIFYREGISTER apidl; /* array of entries to watch*/
-	UINT cidl;		/* number of pidls in array */
+	UINT cidl;		/* number of pidls and dirchangeinfos in array */
 	LONG wEventMask;	/* subscribed events */
 	DWORD dwFlags;		/* client flags */
 	ULONG id;
@@ -128,6 +150,22 @@ static void DeleteNode(LPNOTIFICATIONLIST item)
     UINT i;
 
     TRACE("item=%p\n", item);
+    
+    if (item->dwFlags & SHCNRF_InterruptLevel)
+    {
+        for (i = 0; i < item->cidl; i++)
+            CloseHandle(item->changeinfo[i].handle);
+
+        SetEvent(item->listener_done);
+        WaitForSingleObject(item->listener_thread, INFINITE);
+        CloseHandle(item->listener_thread);
+        CloseHandle(item->listener_ready);
+        CloseHandle(item->listener_done);
+        for (i = 0; i < item->cidl; i++)
+            if (item->changeinfo[i].target_file != NULL)
+                SHFree(item->changeinfo[i].target_file);
+        SHFree(item->changeinfo);
+    }
 
     /* remove item from list */
     list_remove( &item->entry );
@@ -135,6 +173,7 @@ static void DeleteNode(LPNOTIFICATIONLIST item)
     /* free the item */
     for (i=0; i<item->cidl; i++)
         SHFree((LPITEMIDLIST)item->apidl[i].pidl);
+
     SHFree(item->apidl);
     SHFree(item);
 }
@@ -211,6 +250,304 @@ static BOOL notify_recipient(
     return sendmsg_result != 0;
 }
 
+/* imperfect conversion from SHCNE_* event to FILE_NOTIFY_CHANGE_* filter */
+static DWORD notify_filter_for_event(LONG event)
+{
+    DWORD notify_filter = 0;
+
+    if (event & (SHCNE_CREATE |
+                 SHCNE_DELETE |
+                 SHCNE_RENAMEFOLDER |
+                 SHCNE_RENAMEITEM |
+                 SHCNE_RMDIR))
+        notify_filter |= FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_DIR_NAME;
+    if (event & SHCNE_ATTRIBUTES)
+        notify_filter |= FILE_NOTIFY_CHANGE_ATTRIBUTES;
+    if (event & SHCNE_FREESPACE)
+        notify_filter |= FILE_NOTIFY_CHANGE_SIZE;
+
+    return notify_filter;
+}
+
+static FILE_NOTIFY_INFORMATION *dirchangeinfo_buffer(DIRCHANGEINFO *changeinfo)
+{
+    /* at least in real win32, there's a race condition when using the same buffer in
+       subsequent calls to ReadDirectoryChangesW */
+    if (changeinfo->use_buffer1)
+        return (FILE_NOTIFY_INFORMATION *)changeinfo->buffer1;
+    else
+        return (FILE_NOTIFY_INFORMATION *)changeinfo->buffer2;
+}
+
+static void CALLBACK directory_change_completion(DWORD, DWORD, OVERLAPPED *);
+
+static BOOLEAN dirchangeinfo_register_callback(DIRCHANGEINFO *changeinfo)
+{
+    return ReadDirectoryChangesW(
+        changeinfo->handle,
+        dirchangeinfo_buffer(changeinfo),
+        sizeof(changeinfo->buffer1),
+        changeinfo->watch->fRecursive && (changeinfo->nl->dwFlags & SHCNRF_RecursiveInterrupt),
+        notify_filter_for_event(changeinfo->nl->wEventMask),
+        NULL,
+        (OVERLAPPED *)changeinfo,
+        directory_change_completion
+    );
+}
+
+/* appends path2 onto the end of path1 (non-null-terminated strings) */
+static BOOLEAN append_path(
+    WCHAR *path1, ULONG path1_len,
+    WCHAR *path2, ULONG path2_len)
+{
+    /* first, see if path1 needs a slash */
+    if (path1[path1_len-1] != '\\')
+        path1_len += 1;
+
+    /* the lengths here exclude the null terminator */
+    if (path1_len + path2_len + 1 > MAX_PATH)
+        return FALSE;
+
+    /* make sure path1 has its slash */
+    path1[path1_len-1] = '\\';
+
+    CopyMemory(path1+path1_len, path2, path2_len*sizeof(WCHAR));
+    path1[path1_len + path2_len] = '\0';
+    return TRUE;
+}
+
+static FILE_NOTIFY_INFORMATION *increment_fni(
+    FILE_NOTIFY_INFORMATION *fni,
+    WCHAR *base_path,
+    ULONG base_path_len)
+{
+    if (fni->NextEntryOffset == 0)
+        return NULL;
+
+    fni = (FILE_NOTIFY_INFORMATION *)(((BYTE *)fni) + fni->NextEntryOffset);
+
+    if (!append_path(base_path, base_path_len, fni->FileName, fni->FileNameLength/sizeof(WCHAR)))
+    {
+        TRACE("notification path too long\n");
+        return NULL;
+    }
+
+    return fni;
+}
+
+static WCHAR *get_file_spec_w(WCHAR *path)
+{
+    LONG i, path_len = strlenW(path);
+
+    for (i = path_len-1; i >= 0; i--)
+        if (path[i] == '\\')
+            return path + i + 1;
+
+    return NULL;
+}
+
+static void CALLBACK directory_change_completion(
+    DWORD error,
+    DWORD bytes,
+    OVERLAPPED *overlapped)
+{
+    /* the OVERLAPPED should be the first member of DIRCHANGEINFO */
+    DIRCHANGEINFO *changeinfo = (DIRCHANGEINFO *)overlapped;
+    FILE_NOTIFY_INFORMATION *change;
+    HANDLE shared_data = NULL;
+    WCHAR path[MAX_PATH];
+    ULONG path_len;
+    LPITEMIDLIST pidls[2];
+    BOOL notify_succeeded = TRUE;
+
+    if (error != ERROR_SUCCESS || bytes == 0)
+    {
+        if (error == ERROR_OPERATION_ABORTED || error == 0xFFFFFFF6)
+            TRACE("ReadDirectoryChangesW aborted\n");
+        else
+            TRACE("error in ReadDirectoryChangesW completion function %i\n", error);
+        return;
+    }
+
+    ZeroMemory(pidls, sizeof(pidls));
+    CopyMemory(path, changeinfo->listen_path, sizeof(changeinfo->listen_path));
+    path_len = strlenW(path);
+
+    change = dirchangeinfo_buffer(changeinfo);
+    if (!append_path(path, path_len, change->FileName, change->FileNameLength/sizeof(WCHAR)))
+    {
+        TRACE("notification path too long\n");
+        return;
+    }
+
+    while (change != NULL)
+    {
+        LONG event_id = 0;
+        BOOL skip = FALSE;
+        notify_succeeded = TRUE;
+
+        /* skip notifications that we aren't interested in */
+        if (changeinfo->target_file != NULL)
+        {
+            WCHAR *file_spec = get_file_spec_w(path);
+            TRACE("changed file spec: %s (from %s); target file spec: %s\n",
+                  wine_dbgstr_w(file_spec), wine_dbgstr_w(path),
+                  wine_dbgstr_w(changeinfo->target_file));
+            if (file_spec == NULL || strcmpW(file_spec, changeinfo->target_file) != 0)
+                skip = TRUE;
+        }
+        
+        if (!skip)
+        {
+            switch (change->Action)
+            {
+            case FILE_ACTION_ADDED:
+                event_id = SHCNE_CREATE;
+                pidls[0] = ILCreateFromPathW(path);
+                break;
+
+            case FILE_ACTION_REMOVED:
+                event_id = SHCNE_DELETE;
+                pidls[0] = SHSimpleIDListFromPathW(path);
+                break;
+
+            case FILE_ACTION_MODIFIED:
+                event_id = SHCNE_ATTRIBUTES;
+                pidls[0] = ILCreateFromPathW(path);
+                break;
+
+            case FILE_ACTION_RENAMED_OLD_NAME:
+                pidls[0] = SHSimpleIDListFromPathW(path);
+                change = increment_fni(change, path, path_len);
+                if (change == NULL || change->Action != FILE_ACTION_RENAMED_NEW_NAME)
+                {
+                    ERR("FILE_ACTION_RENAMED_OLD_NAME showed up without FILE_ACTION_RENAMED_NEW_NAME\n");
+                    skip = TRUE;
+                    break;
+                }
+                pidls[1] = SHSimpleIDListFromPathW(path);
+
+                event_id = PathIsDirectoryW(path) ? SHCNE_RENAMEFOLDER : SHCNE_RENAMEITEM;
+                break;
+
+            default:
+                TRACE("unexpected file action 0x%x\n", change->Action);
+            }
+        }
+
+        if (!skip)
+        {
+            /* send notification message */
+            if (pidls[0])
+            {
+                TRACE("sending 0x%x for %s\n", event_id, wine_dbgstr_w(path));
+                notify_succeeded = notify_recipient(changeinfo->nl->hwnd, changeinfo->nl->uMsg,
+                                        changeinfo->nl->dwFlags, event_id, pidls, &shared_data);
+            }
+            else
+                TRACE("pidl didn't get set for 0x%x %s\n", event_id, wine_dbgstr_w(path));
+        }
+
+        if (pidls[0])
+            SHFree(pidls[0]);
+        if (pidls[1])
+            SHFree(pidls[1]);
+        ZeroMemory(pidls, sizeof(pidls));
+
+        /* if notify_recipient returned FALSE, we want to just clean up and quit */
+        if (notify_succeeded)
+            change = increment_fni(change, path, path_len);
+        else {
+            TRACE("notifications canceled\n");
+            change = NULL;
+        }
+    }
+
+    SHFreeShared(shared_data, GetCurrentProcessId());
+    shared_data = NULL;
+
+    changeinfo->use_buffer1 = !changeinfo->use_buffer1;
+
+    if (notify_succeeded && !dirchangeinfo_register_callback(changeinfo))
+        TRACE("failed to reissue ReadDirectoryChangesW\n");
+}
+
+static void CALLBACK directory_change_listener_thread(void *parameter)
+{
+    NOTIFICATIONLIST *item = (NOTIFICATIONLIST *)parameter;
+    UINT i;
+    DWORD wait_result;
+
+    /* set up ReadDirectoryChangesW for each idlist being watched */
+    for (i = 0; i < item->cidl; i++)
+    {
+        BOOL path_is_directory;
+        BOOL result;
+
+        item->changeinfo[i].nl = item;
+        item->changeinfo[i].watch = &item->apidl[i];
+        if (!SHGetPathFromIDListW(item->apidl[i].pidl, item->changeinfo[i].listen_path))
+        {
+            TRACE("failed to get path for pidl (%p) (%s)\n", item->apidl[i].pidl, NodeName(item));
+            continue;
+        }
+
+        /* we need to see if this path points to a directory or not */
+        path_is_directory = PathIsRootW(item->changeinfo[i].listen_path)
+                         || PathIsDirectoryW(item->changeinfo[i].listen_path);
+
+        if (!path_is_directory)
+        {
+            /* save the target file name */
+            WCHAR *file_spec = get_file_spec_w(item->changeinfo[i].listen_path);
+            if (file_spec == NULL || file_spec == item->changeinfo[i].listen_path)
+            {
+                TRACE("failed to remove filename from %s\n", wine_dbgstr_w(item->changeinfo[i].listen_path));
+                continue;
+            }
+
+            item->changeinfo[i].target_file = (WCHAR *)SHAlloc(MAX_PATH);
+            strcpyW(item->changeinfo[i].target_file, file_spec);
+            PathRemoveFileSpecW(item->changeinfo[i].listen_path);
+
+            TRACE("listen path: %s, target file: %s\n",
+                wine_dbgstr_w(item->changeinfo[i].listen_path),
+                wine_dbgstr_w(item->changeinfo[i].target_file));
+        }
+
+        /* grab a handle for the directory to watch */
+        item->changeinfo[i].handle = CreateFileW(
+            item->changeinfo[i].listen_path,
+            FILE_LIST_DIRECTORY | SYNCHRONIZE,
+            FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+            NULL,
+            OPEN_EXISTING,
+            FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED,
+            NULL);
+
+        if (item->changeinfo[i].handle == INVALID_HANDLE_VALUE)
+        {
+            TRACE("failed to retrieve handle for %s\n", wine_dbgstr_w(item->changeinfo[i].listen_path));
+            continue;
+        }
+
+        result = dirchangeinfo_register_callback(&item->changeinfo[i]);
+
+        if (!result)
+            ERR("ReadDirectoryChangesW failed on %s\n", wine_dbgstr_w(item->changeinfo[i].listen_path));
+    } /* set up directory change listener */
+
+    /* notify main thread that we're done setting up */
+    SetEvent(item->listener_ready);
+
+    /* remain in an alertable wait state while the completion functions roll in */
+    do {
+        wait_result = WaitForSingleObjectEx(item->listener_done, INFINITE, TRUE);
+    } while (wait_result == WAIT_IO_COMPLETION);
+
+    TRACE("shutting down change listener\n");
+}
+
 /*************************************************************************
  * SHChangeNotifyRegister			[SHELL32.2]
  *
@@ -253,6 +590,53 @@ SHChangeNotifyRegister(
 
     LeaveCriticalSection(&SHELL32_ChangenotifyCS);
 
+    if (fSources & SHCNRF_InterruptLevel)
+    {
+        HANDLE handles[2];
+        DWORD wait_result;
+
+        TRACE("registering with SHCNRF_InterruptLevel\n");
+
+        item->changeinfo = SHAlloc(sizeof(DIRCHANGEINFO) * cItems);
+        ZeroMemory(item->changeinfo, sizeof(DIRCHANGEINFO) * cItems);
+
+        /* listener_done is set by us when we want the listener to quit */
+        item->listener_done = CreateEventW(NULL, FALSE, FALSE, NULL);
+        /* listener_ready is set by the listener when it's done setting up */
+        handles[0] = item->listener_ready = CreateEventW(NULL, TRUE, FALSE, NULL);
+        handles[1] = item->listener_thread = CreateThread(
+            NULL, 0, (LPTHREAD_START_ROUTINE)directory_change_listener_thread,
+            (void *)item, 0, 0
+        );
+
+        /* wait for the thread to either terminate or signal that it's done setting up */
+        wait_result = WaitForMultipleObjects(2, handles, FALSE, INFINITE);
+
+        switch (wait_result)
+        {
+        case WAIT_OBJECT_0:
+            TRACE("listener thread started\n");
+            break;
+
+        case WAIT_OBJECT_0+1:
+            ERR("listener thread failed to start\n");
+            break;
+        
+        case WAIT_FAILED:
+            ERR("failed to wait for change listener thread: %u\n", GetLastError());
+            break;
+
+        default: break;
+        }
+    }
+    else
+    {
+        item->listener_ready = NULL;
+        item->listener_done = NULL;
+        item->listener_thread = NULL;
+    }
+    
+
     return item->id;
 }
 
@@ -394,7 +778,9 @@ void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID
 
             if (wEventId & ptr->wEventMask)
             {
-                if( !pidl )          /* all ? */
+                if ( !(ptr->dwFlags & SHCNRF_ShellLevel) )
+                    notify = FALSE;
+                else if( !pidl )          /* all ? */
                     notify = TRUE;
                 else if( wEventId & SHCNE_NOITEMEVENTS )
                     notify = TRUE;
-- 
2.20.1




More information about the wine-devel mailing list