Ensure handles are closed when pipe disconnects

Fixes leaks where the pipe handles were not closed, preventing new
connections. Fixes the MONITOR_PIPE context leaking when its thread
is stopped.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
Since error gotos are introduced, have them do the cleanup. The scope of
Pipe and Instance needs to be widened but it keeps the code neater overall.
Also removed the unnecessary newlines in the log messages.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 3e535e4e1b4b51f51c8c354c46d2cf3668193c90..e54f47d665734c71cf6761fb32a7a0de2550096e 100644 (file)
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -469,6 +469,10 @@ PipeThread(
 
     CloseHandle(Overlapped.hEvent);
 
+    CloseHandle(Pipe->Pipe);
+    CloseHandle(Pipe->Thread);
+    free(Pipe);
+
     Log("<====");
 
     return 0;
@@ -494,7 +498,9 @@ ServerThread(
     PMONITOR_CONTEXT    Context = &MonitorContext;
     OVERLAPPED          Overlapped;
     HANDLE              Handle[2];
+    HANDLE              Pipe;
     DWORD               Object;
+    PMONITOR_PIPE       Instance;
     HRESULT             Error;
 
     UNREFERENCED_PARAMETER(Argument);
@@ -513,9 +519,6 @@ ServerThread(
     Handle[1] = Overlapped.hEvent;
 
     for (;;) {
-        HANDLE          Pipe;
-        PMONITOR_PIPE   Instance;
-
         Pipe = CreateNamedPipe(PIPE_NAME,
                                PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
                                PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE,
@@ -525,7 +528,7 @@ ServerThread(
                                0,
                                NULL);
         if (Pipe == INVALID_HANDLE_VALUE)
-            break;
+            goto fail2;
 
         (VOID) ConnectNamedPipe(Pipe,
                                 &Overlapped);
@@ -534,16 +537,16 @@ ServerThread(
                                         Handle,
                                         FALSE,
                                         INFINITE);
-        if (Object == WAIT_OBJECT_0)
+        if (Object == WAIT_OBJECT_0) {
+            CloseHandle(Pipe);
             break;
+        }
 
         ResetEvent(Overlapped.hEvent);
 
         Instance = (PMONITOR_PIPE)malloc(sizeof(MONITOR_PIPE));
-        if (Instance == NULL) {
-            CloseHandle(Pipe);
-            break;
-        }
+        if (Instance == NULL)
+            goto fail3;
 
         __InitializeListHead(&Instance->ListEntry);
         Instance->Pipe = Pipe;
@@ -554,11 +557,8 @@ ServerThread(
                                         Instance,
                                         0,
                                         NULL);
-        if (Instance->Thread == INVALID_HANDLE_VALUE) {
-            free(Instance);
-            CloseHandle(Pipe);
-            break;
-        }
+        if (Instance->Thread == INVALID_HANDLE_VALUE)
+            goto fail4;
     }
 
     CloseHandle(Overlapped.hEvent);
@@ -567,6 +567,19 @@ ServerThread(
 
     return 0;
 
+fail4:
+    Log("fail4");
+
+    free(Instance);
+
+fail3:
+    Log("fail3");
+
+    CloseHandle(Pipe);
+
+fail2:
+    Log("fail2");
+
 fail1:
     Error = GetLastError();