[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[win-pv-devel] [PATCH 4/4] Handle DBT_DEVICEQUERYREMOVEFAILED



Split ConsoleCreate/ConsoleDestroy so that a query remove can close all
device handles, and re-open them is a query remove failed is received.
This allows the named pipes to keep operating, discarding any data
whilst the device is being query removed.
Also fixes several issues with the threading model and leaking handles.

Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
 src/monitor/monitor.c | 418 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 308 insertions(+), 110 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 1912047..d871af7 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -70,6 +70,7 @@ typedef struct _MONITOR_CONSOLE {
     LIST_ENTRY              ListEntry;
     PWCHAR                  DevicePath;
     HANDLE                  DeviceHandle;
+    BOOL                    DeviceHandleOpen;
     HDEVNOTIFY              DeviceNotification;
     PCHAR                   DeviceName; // protocol and instance?
     HANDLE                  ExecutableThread;
@@ -387,9 +388,14 @@ ConnectionThread(
 
         ResetEvent(Overlapped.hEvent);
 
-        PutString(Console->DeviceHandle,
-                  Buffer,
-                  Length);
+        EnterCriticalSection(&Console->CriticalSection);
+        if (Console->DeviceHandleOpen)
+            PutString(Console->DeviceHandle,
+                      Buffer,
+                      Length);
+        else
+            Log("[NO_DEVICE] %.*hs", Length, Buffer);
+        LeaveCriticalSection(&Console->CriticalSection);
     }
 
     EnterCriticalSection(&Console->CriticalSection);
@@ -460,6 +466,8 @@ ServerThread(
     Log("%s", PipeName);
 
     for (;;) {
+        DWORD           ThreadId;
+
         Pipe = CreateNamedPipe(PipeName,
                                PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
                                PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE,
@@ -496,10 +504,12 @@ ServerThread(
                                           0,
                                           ConnectionThread,
                                           Connection,
-                                          0,
-                                          NULL);
+                                          CREATE_SUSPENDED,
+                                          &ThreadId);
         if (Connection->Thread == NULL)
             goto fail5;
+
+        ResumeThread(Connection->Thread);
     }
 
     CloseHandle(Overlapped.hEvent);
@@ -839,6 +849,138 @@ fail1:
     return 1;
 }
 
+static BOOL
+ConsoleOpen(
+    IN  PMONITOR_CONSOLE    Console
+    )
+{
+    HANDLE                  DeviceHandle;
+    CHAR                    DeviceName[MAX_PATH];
+    DWORD                   Bytes;
+    DWORD                   ThreadId;
+    BOOL                    Success;
+
+    Log("====> %ws", Console->DevicePath);
+
+    if (Console->DeviceHandleOpen)
+        goto fail1;
+
+    DeviceHandle = CreateFileW(Console->DevicePath,
+                               GENERIC_READ | GENERIC_WRITE,
+                               FILE_SHARE_READ | FILE_SHARE_WRITE,
+                               NULL,
+                               OPEN_EXISTING,
+                               FILE_ATTRIBUTE_NORMAL,
+                               NULL);
+    if (DeviceHandle == INVALID_HANDLE_VALUE)
+        goto fail2;
+
+    // check re-open reuses a handle
+    if ((Console->DeviceHandle != INVALID_HANDLE_VALUE) &&
+        (Console->DeviceHandle != DeviceHandle))
+        Log("HANDLE changed! %p -> %p",
+            (VOID*)Console->DeviceHandle,
+            (VOID*)DeviceHandle);
+
+    EnterCriticalSection(&Console->CriticalSection);
+    Console->DeviceHandle = DeviceHandle;
+    Console->DeviceHandleOpen = TRUE;
+    LeaveCriticalSection(&Console->CriticalSection);
+
+    if (Console->DeviceName == NULL) {
+        Success = DeviceIoControl(Console->DeviceHandle,
+                                  IOCTL_XENCONS_GET_NAME,
+                                  NULL,
+                                  0,
+                                  DeviceName,
+                                  sizeof(DeviceName),
+                                  &Bytes,
+                                  NULL);
+        if (!Success)
+            goto fail3;
+
+        DeviceName[MAX_PATH - 1] = '\0';
+
+        Console->DeviceName = _strdup(DeviceName);
+        if (Console->DeviceName == NULL)
+            goto fail4;
+    }
+
+    ECHO(Console->DeviceHandle, "\r\n[ATTACHED]\r\n");
+
+    Console->DeviceThread = CreateThread(NULL,
+                                         0,
+                                         DeviceThread,
+                                         Console,
+                                         CREATE_SUSPENDED,
+                                         &ThreadId);
+    if (Console->DeviceThread == NULL)
+        goto fail5;
+
+    ResumeThread(Console->DeviceThread);
+
+    Log("<==== %s", Console->DeviceName);
+
+    return TRUE;
+
+fail5:
+    Log("fail5");
+
+    ECHO(Console->DeviceHandle, "\r\n[DETATCHED]\r\n");
+
+    free(Console->DeviceName);
+    Console->DeviceName = NULL;
+
+fail4:
+    Log("fail4");
+
+fail3:
+    Log("fail3");
+
+    EnterCriticalSection(&Console->CriticalSection);
+    Console->DeviceHandleOpen = FALSE;
+    CloseHandle(Console->DeviceHandle);
+    LeaveCriticalSection(&Console->CriticalSection);
+
+fail2:
+    Log("fail2");
+
+fail1:
+    Log("fail1");
+
+    return FALSE;
+}
+
+static VOID
+ConsoleClose(
+    IN  PMONITOR_CONSOLE    Console
+    )
+{
+    Log("====> %s", Console->DeviceName);
+
+    if (!Console->DeviceHandleOpen)
+        goto fail1;
+
+    SetEvent(Console->DeviceEvent);
+    WaitForSingleObject(Console->DeviceThread, INFINITE);
+
+    CloseHandle(Console->DeviceThread);
+    Console->DeviceThread = NULL;
+
+    ECHO(Console->DeviceHandle, "\r\n[DETACHED]\r\n");
+
+    EnterCriticalSection(&Console->CriticalSection);
+    Console->DeviceHandleOpen = FALSE;
+    CloseHandle(Console->DeviceHandle);
+    LeaveCriticalSection(&Console->CriticalSection);
+
+    Log("<==== %s", Console->DeviceName);
+    return;
+
+fail1:
+    Log("fail1");
+}
+
 static PMONITOR_CONSOLE
 ConsoleCreate(
     IN  PWCHAR              DevicePath
@@ -847,10 +989,8 @@ ConsoleCreate(
     PMONITOR_CONTEXT        Context = &MonitorContext;
     PMONITOR_CONSOLE        Console;
     DEV_BROADCAST_HANDLE    Handle;
-    CHAR                    DeviceName[MAX_PATH];
-    DWORD                   Bytes;
-    BOOL                    Success;
     HRESULT                 Error;
+    DWORD                   ThreadId;
 
     Log("====> %ws", DevicePath);
 
@@ -862,39 +1002,36 @@ ConsoleCreate(
     __InitializeListHead(&Console->ListHead);
     __InitializeListHead(&Console->ListEntry);
     InitializeCriticalSection(&Console->CriticalSection);
+    Console->DeviceHandleOpen = FALSE;
+    Console->DeviceHandle = INVALID_HANDLE_VALUE;
 
-    Console->DevicePath = _wcsdup(DevicePath);
-    if (Console->DevicePath == NULL)
+    Console->DeviceEvent = CreateEvent(NULL,
+                                       TRUE,
+                                       FALSE,
+                                       NULL);
+    if (Console->DeviceEvent == NULL)
         goto fail2;
 
-    Console->DeviceHandle = CreateFileW(DevicePath,
-                                        GENERIC_READ | GENERIC_WRITE,
-                                        FILE_SHARE_READ | FILE_SHARE_WRITE,
-                                        NULL,
-                                        OPEN_EXISTING,
-                                        FILE_ATTRIBUTE_NORMAL,
-                                        NULL);
-    if (Console->DeviceHandle == INVALID_HANDLE_VALUE)
+    Console->ServerEvent = CreateEvent(NULL,
+                                       TRUE,
+                                       FALSE,
+                                       NULL);
+    if (Console->ServerEvent == NULL)
         goto fail3;
 
-    Success = DeviceIoControl(Console->DeviceHandle,
-                              IOCTL_XENCONS_GET_NAME,
-                              NULL,
-                              0,
-                              DeviceName,
-                              sizeof(DeviceName),
-                              &Bytes,
-                              NULL);
-    if (!Success)
+    Console->ExecutableEvent = CreateEvent(NULL,
+                                           TRUE,
+                                           FALSE,
+                                           NULL);
+    if (Console->ExecutableEvent == NULL)
         goto fail4;
 
-    DeviceName[MAX_PATH - 1] = '\0';
-
-    Console->DeviceName = _strdup(DeviceName);
-    if (Console->DeviceName == NULL)
+    Console->DevicePath = _wcsdup(DevicePath);
+    if (Console->DevicePath == NULL)
         goto fail5;
 
-    ECHO(Console->DeviceHandle, "\r\n[ATTACHED]\r\n");
+    if (!ConsoleOpen(Console))
+        goto fail6;
 
     ZeroMemory(&Handle, sizeof (Handle));
     Handle.dbch_size = sizeof (Handle);
@@ -906,118 +1043,80 @@ ConsoleCreate(
                                     &Handle,
                                     DEVICE_NOTIFY_SERVICE_HANDLE);
     if (Console->DeviceNotification == NULL)
-        goto fail6;
-
-    Console->DeviceEvent = CreateEvent(NULL,
-                                       TRUE,
-                                       FALSE,
-                                       NULL);
-    if (Console->DeviceEvent == NULL)
         goto fail7;
 
-    Console->DeviceThread = CreateThread(NULL,
-                                         0,
-                                         DeviceThread,
-                                         Console,
-                                         0,
-                                         NULL);
-    if (Console->DeviceThread == NULL)
-        goto fail8;
-
-    Console->ServerEvent = CreateEvent(NULL,
-                                       TRUE,
-                                       FALSE,
-                                       NULL);
-    if (Console->ServerEvent == NULL)
-        goto fail9;
-
     Console->ServerThread = CreateThread(NULL,
                                          0,
                                          ServerThread,
                                          Console,
-                                         0,
-                                         NULL);
+                                         CREATE_SUSPENDED,
+                                         &ThreadId);
     if (Console->ServerThread == NULL)
-        goto fail10;
+        goto fail8;
 
-    Console->ExecutableEvent = CreateEvent(NULL,
-                                           TRUE,
-                                           FALSE,
-                                           NULL);
-    if (Console->ExecutableEvent == NULL)
-        goto fail11;
+    ResumeThread(Console->ServerThread);
 
     Console->ExecutableThread = CreateThread(NULL,
                                              0,
                                              ExecutableThread,
                                              Console,
-                                             0,
-                                             NULL);
+                                             CREATE_SUSPENDED,
+                                             &ThreadId);
     if (Console->ExecutableThread == NULL)
-        goto fail12;
+        goto fail9;
+
+    ResumeThread(Console->ExecutableThread);
 
     Log("<==== %s", Console->DeviceName);
 
     return Console;
 
-fail12:
-    Log("fail12");
-
-    CloseHandle(Console->ExecutableEvent);
-    Console->ExecutableEvent = NULL;
-
-fail11:
-    Log("fail11");
+fail9:
+    Log("fail9");
 
     SetEvent(Console->ServerEvent);
     WaitForSingleObject(Console->ServerThread, INFINITE);
 
-fail10:
-    Log("fail10");
-
-    CloseHandle(Console->ServerEvent);
-    Console->ServerEvent = NULL;
-
-fail9:
-    Log("fail9");
-
-    SetEvent(Console->DeviceEvent);
-    WaitForSingleObject(Console->DeviceThread, INFINITE);
+    CloseHandle(Console->ServerThread);
+    Console->ServerThread = NULL;
 
 fail8:
     Log("fail8");
 
-    CloseHandle(Console->DeviceEvent);
-    Console->DeviceEvent = NULL;
+    UnregisterDeviceNotification(Console->DeviceNotification);
+    Console->DeviceNotification = NULL;
 
 fail7:
     Log("fail7");
 
-    UnregisterDeviceNotification(Console->DeviceNotification);
-    Console->DeviceNotification = NULL;
+    EnterCriticalSection(&Console->CriticalSection);
+    ConsoleClose(Console);
+    Console->DeviceHandle = INVALID_HANDLE_VALUE;
+
 
 fail6:
     Log("fail6");
 
-    ECHO(Console->DeviceHandle, "\r\n[DETACHED]\r\n");
-
     free(Console->DevicePath);
     Console->DevicePath = NULL;
 
 fail5:
     Log("fail5");
 
+    CloseHandle(Console->ExecutableEvent);
+    Console->ExecutableEvent = NULL;
+
 fail4:
     Log("fail4");
 
-    CloseHandle(Console->DeviceHandle);
-    Console->DeviceHandle = INVALID_HANDLE_VALUE;
+    CloseHandle(Console->ServerEvent);
+    Console->ServerEvent = NULL;
 
 fail3:
     Log("fail3");
 
-    free(Console->DevicePath);
-    Console->DevicePath = NULL;
+    CloseHandle(Console->DeviceEvent);
+    Console->DeviceEvent = NULL;
 
 fail2:
     Log("fail2");
@@ -1099,33 +1198,33 @@ ConsoleDestroy(
     SetEvent(Console->ExecutableEvent);
     WaitForSingleObject(Console->ExecutableThread, INFINITE);
 
-    CloseHandle(Console->ExecutableEvent);
-    Console->ExecutableEvent = NULL;
+    CloseHandle(Console->ExecutableThread);
+    Console->ExecutableThread = NULL;
 
     ConsoleWaitForPipes(Console);
 
-    CloseHandle(Console->ServerEvent);
-    Console->ServerEvent = NULL;
-
-    SetEvent(Console->DeviceEvent);
-    WaitForSingleObject(Console->DeviceThread, INFINITE);
-
-    CloseHandle(Console->DeviceEvent);
-    Console->DeviceEvent = NULL;
+    CloseHandle(Console->ServerThread);
+    Console->ServerThread = NULL;
 
     UnregisterDeviceNotification(Console->DeviceNotification);
     Console->DeviceNotification = NULL;
 
-    ECHO(Console->DeviceHandle, "\r\n[DETACHED]\r\n");
+    free(Console->DeviceName);
+    Console->DeviceName = NULL;
 
     free(Console->DevicePath);
     Console->DevicePath = NULL;
 
-    CloseHandle(Console->DeviceHandle);
     Console->DeviceHandle = INVALID_HANDLE_VALUE;
 
-    free(Console->DevicePath);
-    Console->DevicePath = NULL;
+    CloseHandle(Console->ExecutableEvent);
+    Console->ExecutableEvent = NULL;
+
+    CloseHandle(Console->ServerEvent);
+    Console->ServerEvent = NULL;
+
+    CloseHandle(Console->DeviceEvent);
+    Console->DeviceEvent = NULL;
 
     DeleteCriticalSection(&Console->CriticalSection);
     ZeroMemory(&Console->ListHead, sizeof(LIST_ENTRY));
@@ -1136,6 +1235,88 @@ ConsoleDestroy(
     Log("<====");
 }
 
+static BOOL
+MonitorOpen(
+    IN  HANDLE          DeviceHandle
+    )
+{
+    PMONITOR_CONTEXT    Context = &MonitorContext;
+    PMONITOR_CONSOLE    Console;
+    PLIST_ENTRY         ListEntry;
+
+    Log("=====> 0x%p", DeviceHandle);
+
+    EnterCriticalSection(&Context->CriticalSection);
+    for (ListEntry = Context->ListHead.Flink;
+        ListEntry != &Context->ListHead;
+        ListEntry = ListEntry->Flink) {
+        Console = CONTAINING_RECORD(ListEntry,
+            MONITOR_CONSOLE,
+            ListEntry);
+
+        if (Console->DeviceHandle == DeviceHandle)
+            goto found;
+    }
+    LeaveCriticalSection(&Context->CriticalSection);
+
+    Log("DeviceHandle 0x%p not found", DeviceHandle);
+
+    return FALSE;
+
+found:
+    LeaveCriticalSection(&Context->CriticalSection);
+
+    if (!ConsoleOpen(Console))
+        goto fail1;
+
+    Log("<=====");
+
+    return TRUE;
+
+fail1:
+    Log("fail1");
+
+    return FALSE;
+}
+
+static BOOL
+MonitorClose(
+    IN  HANDLE          DeviceHandle
+    )
+{
+    PMONITOR_CONTEXT    Context = &MonitorContext;
+    PMONITOR_CONSOLE    Console;
+    PLIST_ENTRY         ListEntry;
+
+    Log("=====> 0x%p", DeviceHandle);
+
+    EnterCriticalSection(&Context->CriticalSection);
+    for (ListEntry = Context->ListHead.Flink;
+        ListEntry != &Context->ListHead;
+        ListEntry = ListEntry->Flink) {
+        Console = CONTAINING_RECORD(ListEntry,
+            MONITOR_CONSOLE,
+            ListEntry);
+
+        if (Console->DeviceHandle == DeviceHandle)
+            goto found;
+    }
+    LeaveCriticalSection(&Context->CriticalSection);
+
+    Log("DeviceHandle 0x%p not found", DeviceHandle);
+
+    return FALSE;
+
+found:
+    LeaveCriticalSection(&Context->CriticalSection);
+
+    ConsoleClose(Console);
+
+    Log("<=====");
+
+    return TRUE;
+}
+
 static BOOL
 MonitorAdd(
     IN  PWCHAR          DevicePath
@@ -1342,6 +1523,7 @@ MonitorRemoveAll(
 
         LeaveCriticalSection(&Context->CriticalSection);
 
+        ConsoleClose(Console);
         ConsoleDestroy(Console);
     }
     LeaveCriticalSection(&Context->CriticalSection);
@@ -1386,7 +1568,23 @@ MonitorCtrlHandlerEx(
             }
             break;
 
+        case DBT_DEVICEQUERYREMOVEFAILED:
+            if (Header->dbch_devicetype == DBT_DEVTYP_HANDLE) {
+                PDEV_BROADCAST_HANDLE Device = EventData;
+
+                MonitorOpen(Device->dbch_handle);
+                // should check return - Open can fail!
+            }
+            break;
+
         case DBT_DEVICEQUERYREMOVE:
+            if (Header->dbch_devicetype == DBT_DEVTYP_HANDLE) {
+                PDEV_BROADCAST_HANDLE Device = EventData;
+
+                MonitorClose(Device->dbch_handle);
+            }
+            break;
+
         case DBT_DEVICEREMOVEPENDING:
         case DBT_DEVICEREMOVECOMPLETE:
             if (Header->dbch_devicetype == DBT_DEVTYP_HANDLE) {
-- 
2.16.2.windows.1


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.