From 8aea5b82daca06cb9e24ba29367bb0985e0d5764 Mon Sep 17 00:00:00 2001
From: Romain Vimont <rom@rom1v.com>
Date: Sun, 14 Nov 2021 19:29:54 +0100
Subject: [PATCH] Inherit only specific handles on Windows

To be able to communicate with a child process via stdin, stdout and
stderr, the CreateProcess() parameter bInheritHandles must be set to
TRUE. But this causes *all* handles to be inherited, including sockets.

As a result, the server socket is inherited by the process running adb
to execute the server on the device, so it may not be closed properly,
causing other instances of scrcpy to fail.

To fix the issue, use an extended API to explicitly set the HANDLE to
inherit:
 - <https://stackoverflow.com/a/28185363/1987178>
 - <https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873>

Fixes #2779 <https://github.com/Genymobile/scrcpy/issues/2779>
---
 app/src/sys/win/process.c | 89 +++++++++++++++++++++++++++++++++++----
 1 file changed, 80 insertions(+), 9 deletions(-)

diff --git a/app/src/sys/win/process.c b/app/src/sys/win/process.c
index 4dcd542ee6..fe04e0aa15 100644
--- a/app/src/sys/win/process.c
+++ b/app/src/sys/win/process.c
@@ -1,5 +1,10 @@
+// <https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873>
+#define _WIN32_WINNT 0x0600 // For extended process API
+
 #include "util/process.h"
 
+#include <processthreadsapi.h>
+
 #include <assert.h>
 
 #include "util/log.h"
@@ -26,6 +31,17 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
                      HANDLE *pin, HANDLE *pout, HANDLE *perr) {
     enum sc_process_result ret = SC_PROCESS_ERROR_GENERIC;
 
+    unsigned handle_count = 0;
+    if (pin) {
+        ++handle_count;
+    }
+    if (pout) {
+        ++handle_count;
+    }
+    if (perr) {
+        ++handle_count;
+    }
+
     SECURITY_ATTRIBUTES sa;
     sa.nLength = sizeof(SECURITY_ATTRIBUTES);
     sa.lpSecurityDescriptor = NULL;
@@ -65,20 +81,63 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
         }
     }
 
-    STARTUPINFOW si;
+    STARTUPINFOEXW si;
     PROCESS_INFORMATION pi;
     memset(&si, 0, sizeof(si));
-    si.cb = sizeof(si);
-    if (pin || pout || perr) {
-        si.dwFlags = STARTF_USESTDHANDLES;
+    si.StartupInfo.cb = sizeof(si);
+
+    LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
+    if (handle_count) {
+        si.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
+        if (pin) {
+            si.StartupInfo.hStdInput = stdin_read_handle;
+        }
+        if (pout) {
+            si.StartupInfo.hStdOutput = stdout_write_handle;
+        }
+        if (perr) {
+            si.StartupInfo.hStdError = stderr_write_handle;
+        }
+
+        SIZE_T size = 0;
+        // Call it once to know the required buffer size
+        BOOL ok =
+            InitializeProcThreadAttributeList(NULL, handle_count, 0, &size)
+                || GetLastError() == ERROR_INSUFFICIENT_BUFFER;
+        if (!ok) {
+            goto error_close_stderr;
+        }
+
+        lpAttributeList = malloc(size);
+        if (!lpAttributeList) {
+            goto error_close_stderr;
+        }
+
+        ok = InitializeProcThreadAttributeList(lpAttributeList, handle_count, 0,
+                                               &size);
+        if (!ok) {
+            free(lpAttributeList);
+            goto error_close_stderr;
+        }
+
+        // Pass explicitly the HANDLEs that must be inherited
+        HANDLE handles[3];
+        unsigned i = 0;
         if (pin) {
-            si.hStdInput = stdin_read_handle;
+            handles[i++] = stdin_read_handle;
         }
         if (pout) {
-            si.hStdOutput = stdout_write_handle;
+            handles[i++] = stdout_write_handle;
         }
         if (perr) {
-            si.hStdError = stderr_write_handle;
+            handles[i++] = stderr_write_handle;
+        }
+        ok = UpdateProcThreadAttribute(lpAttributeList, 0,
+                                       PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+                                       handles, handle_count * sizeof(HANDLE),
+                                       NULL, NULL);
+        if (!ok) {
+            goto error_free_attribute_list;
         }
     }
 
@@ -95,8 +154,11 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
         goto error_close_stderr;
     }
 
-    if (!CreateProcessW(NULL, wide, NULL, NULL, TRUE, 0, NULL, NULL, &si,
-                        &pi)) {
+    si.lpAttributeList = lpAttributeList;
+    BOOL bInheritHandles = handle_count > 0;
+    DWORD dwCreationFlags = handle_count > 0 ? EXTENDED_STARTUPINFO_PRESENT : 0;
+    if (!CreateProcessW(NULL, wide, NULL, NULL, bInheritHandles,
+                        dwCreationFlags, NULL, NULL, &si.StartupInfo, &pi)) {
         free(wide);
         *handle = NULL;
 
@@ -106,6 +168,10 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
         goto error_close_stderr;
     }
 
+    if (lpAttributeList) {
+        free(lpAttributeList);
+    }
+
     // These handles are used by the child process, close them for this process
     if (pin) {
         CloseHandle(stdin_read_handle);
@@ -122,6 +188,11 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
 
     return SC_PROCESS_SUCCESS;
 
+error_free_attribute_list:
+    if (lpAttributeList) {
+        DeleteProcThreadAttributeList(lpAttributeList);
+        free(lpAttributeList);
+    }
 error_close_stderr:
     if (perr) {
         CloseHandle(*perr);