Compare commits

...

6 Commits

Author SHA1 Message Date
Romain Vimont
9cb14b5166 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 was inherited by the process running adb
to execute the server on the device, so it could not be closed properly,
causing other scrcpy instances to fail.

To fix the issue, use an extended API to explicitly set the HANDLEs 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>
PR #2783 <https://github.com/Genymobile/scrcpy/pull/2783>
2021-11-15 10:13:30 +01:00
Romain Vimont
9cb8766220 Factorize resource release after CreateProcess()
Free the wide characters string in all cases before checking for errors.
2021-11-15 07:49:01 +01:00
Romain Vimont
fd4ec784e0 Remove useless assignments on error
Leave the output parameter untouched on error.
2021-11-14 22:53:49 +01:00
Romain Vimont
52cebe1597 Fix Windows sc_pipe function names
The implementation name was incorrect (it was harmless, because they are
not used on Windows).
2021-11-14 22:41:38 +01:00
Romain Vimont
6a27062f48 Stop connection attempts if interrupted
If the interruptor is interrupted, every network call will fail, but the
retry-on-error mechanism must also be stopped.
2021-11-14 15:40:59 +01:00
Romain Vimont
739ff9dce0 Fix compilation errors with old SDL versions
SDL_PixelFormatEnum has been introduced in SDL 2.0.10:
<cc6a8ac87e>

SDL_PIXELFORMAT_BGR444 has been introduced in SDL 2.0.12:
<a1c11854f2>

Fixes #2777 <https://github.com/Genymobile/scrcpy/issues/2777>
PR #2781 <https://github.com/Genymobile/scrcpy/pull/2781>

Reviewed-by: Yu-Chen Lin <npes87184@gmail.com>
2021-11-14 15:37:30 +01:00
3 changed files with 94 additions and 19 deletions

View File

@@ -158,6 +158,12 @@ free_ctx:
return result;
}
#if !SDL_VERSION_ATLEAST(2, 0, 10)
// SDL_PixelFormatEnum has been introduced in SDL 2.0.10. Use int for older SDL
// versions.
typedef int SDL_PixelFormatEnum;
#endif
static SDL_PixelFormatEnum
to_sdl_pixel_format(enum AVPixelFormat fmt) {
switch (fmt) {
@@ -172,7 +178,9 @@ to_sdl_pixel_format(enum AVPixelFormat fmt) {
case AV_PIX_FMT_BGR565BE: return SDL_PIXELFORMAT_BGR565;
case AV_PIX_FMT_BGR555BE: return SDL_PIXELFORMAT_BGR555;
case AV_PIX_FMT_RGB444BE: return SDL_PIXELFORMAT_RGB444;
#if SDL_VERSION_ATLEAST(2, 0, 12)
case AV_PIX_FMT_BGR444BE: return SDL_PIXELFORMAT_BGR444;
#endif
case AV_PIX_FMT_PAL8: return SDL_PIXELFORMAT_INDEX8;
default: return SDL_PIXELFORMAT_UNKNOWN;
}

View File

@@ -234,6 +234,12 @@ connect_to_server(struct sc_server *server, uint32_t attempts, sc_tick delay) {
net_close(socket);
}
if (sc_intr_is_interrupted(&server->intr)) {
// Stop immediately
break;
}
if (attempts) {
sc_mutex_lock(&server->mutex);
sc_tick deadline = sc_tick_now() + delay;

View File

@@ -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,9 @@ 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;
// Add 1 per non-NULL pointer
unsigned handle_count = !!pin + !!pout + !!perr;
SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.lpSecurityDescriptor = NULL;
@@ -65,45 +73,94 @@ 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);
HANDLE handles[3];
LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
if (handle_count) {
si.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
if (pin) {
si.hStdInput = stdin_read_handle;
si.StartupInfo.hStdInput = stdin_read_handle;
}
if (pout) {
si.hStdOutput = stdout_write_handle;
si.StartupInfo.hStdOutput = stdout_write_handle;
}
if (perr) {
si.hStdError = stderr_write_handle;
si.StartupInfo.hStdError = stderr_write_handle;
}
SIZE_T size;
// Call it once to know the required buffer size
BOOL ok =
InitializeProcThreadAttributeList(NULL, 1, 0, &size)
|| GetLastError() == ERROR_INSUFFICIENT_BUFFER;
if (!ok) {
goto error_close_stderr;
}
lpAttributeList = malloc(size);
if (!lpAttributeList) {
goto error_close_stderr;
}
ok = InitializeProcThreadAttributeList(lpAttributeList, 1, 0, &size);
if (!ok) {
free(lpAttributeList);
goto error_close_stderr;
}
// Explicitly pass the HANDLEs that must be inherited
unsigned i = 0;
if (pin) {
handles[i++] = stdin_read_handle;
}
if (pout) {
handles[i++] = stdout_write_handle;
}
if (perr) {
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;
}
si.lpAttributeList = lpAttributeList;
}
char *cmd = malloc(CMD_MAX_LEN);
if (!cmd || !build_cmd(cmd, CMD_MAX_LEN, argv)) {
*handle = NULL;
goto error_close_stderr;
goto error_free_attribute_list;
}
wchar_t *wide = sc_str_to_wchars(cmd);
free(cmd);
if (!wide) {
LOGC("Could not allocate wide char string");
goto error_close_stderr;
goto error_free_attribute_list;
}
if (!CreateProcessW(NULL, wide, NULL, NULL, TRUE, 0, NULL, NULL, &si,
&pi)) {
free(wide);
*handle = NULL;
BOOL bInheritHandles = handle_count > 0;
DWORD dwCreationFlags = handle_count > 0 ? EXTENDED_STARTUPINFO_PRESENT : 0;
BOOL ok = CreateProcessW(NULL, wide, NULL, NULL, bInheritHandles,
dwCreationFlags, NULL, NULL, &si.StartupInfo, &pi);
free(wide);
if (!ok) {
if (GetLastError() == ERROR_FILE_NOT_FOUND) {
ret = SC_PROCESS_ERROR_MISSING_BINARY;
}
goto error_close_stderr;
goto error_free_attribute_list;
}
if (lpAttributeList) {
DeleteProcThreadAttributeList(lpAttributeList);
free(lpAttributeList);
}
// These handles are used by the child process, close them for this process
@@ -117,11 +174,15 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
CloseHandle(stderr_write_handle);
}
free(wide);
*handle = pi.hProcess;
return SC_PROCESS_SUCCESS;
error_free_attribute_list:
if (lpAttributeList) {
DeleteProcThreadAttributeList(lpAttributeList);
free(lpAttributeList);
}
error_close_stderr:
if (perr) {
CloseHandle(*perr);
@@ -168,7 +229,7 @@ sc_process_close(HANDLE handle) {
}
ssize_t
sc_read_pipe(HANDLE pipe, char *data, size_t len) {
sc_pipe_read(HANDLE pipe, char *data, size_t len) {
DWORD r;
if (!ReadFile(pipe, data, len, &r, NULL)) {
return -1;
@@ -177,7 +238,7 @@ sc_read_pipe(HANDLE pipe, char *data, size_t len) {
}
void
sc_close_pipe(HANDLE pipe) {
sc_pipe_close(HANDLE pipe) {
if (!CloseHandle(pipe)) {
LOGW("Cannot close pipe");
}