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 <***@citrix.com>
---
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