-- v3: ntdll: Use posix_spawn() to start the server. ntdll: Factor out generating possible paths to wineserver.
From: Brendan Shanks bshanks@codeweavers.com
--- dlls/ntdll/unix/loader.c | 91 +++++++++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 28 deletions(-)
diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index aa120ae38ec..c184a2d5c4a 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -546,47 +546,80 @@ NTSTATUS exec_wineloader( char **argv, int socketfd, const pe_image_info_t *pe_i
/*********************************************************************** - * exec_wineserver + * next_wineserver_path * - * Exec a new wine server. + * Return possible paths to the server. */ -static void exec_wineserver( char **argv ) +static char *next_wineserver_path( char **pathcopy ) { - char *path; + static int step = 0;
- if (build_dir) + switch (step) { - if (!is_win64) /* look for 64-bit server */ + case 0: + if (build_dir && !is_win64) /* look for 64-bit server */ { - char *loader = realpath_dirname( build_path( build_dir, "loader/wine64" )); + char *loader_relative = build_path( build_dir, "loader/wine64" ); + char *loader = realpath_dirname( loader_relative ); + free( loader_relative ); if (loader) { - argv[0] = build_path( loader, "../server/wineserver" ); - execv( argv[0], argv ); + char *ret = build_path( loader, "../server/wineserver" ); + free( loader ); + step = 1; + return ret; } } - argv[0] = build_path( build_dir, "server/wineserver" ); - execv( argv[0], argv ); - return; - } - - argv[0] = build_path( bin_dir, "wineserver" ); - execv( argv[0], argv ); - - argv[0] = getenv( "WINESERVER" ); - if (argv[0]) execv( argv[0], argv ); + /* fallthrough */ + case 1: + if (build_dir) + { + step = 2; + return build_path( build_dir, "server/wineserver" ); + } + /* fallthrough */ + case 2: + if (build_dir) + return NULL; + /* fallthrough */ + case 3: + step = 4; + return build_path( bin_dir, "wineserver" ); + case 4: + if (getenv( "WINESERVER" )) + { + step = 5; + return strdup( getenv( "WINESERVER" ) ); + } + /* fallthrough */ + case 5: + if (getenv( "PATH" )) + *pathcopy = strdup( getenv( "PATH" ) );
- if ((path = getenv( "PATH" ))) - { - for (path = strtok( strdup( path ), ":" ); path; path = strtok( NULL, ":" )) + step = 6; + if (*pathcopy) { - argv[0] = build_path( path, "wineserver" ); - execvp( argv[0], argv ); + char *path = strtok( *pathcopy, ":" ); + if (path) + return build_path( path, "wineserver" ); + } + /* fallthrough */ + case 6: + if (*pathcopy) + { + char *path = strtok( NULL, ":" ); + if (path) + return build_path( path, "wineserver" ); + else + step = 7; } + /* fallthrough */ + case 7: + step = 8; + return build_path( BINDIR, "wineserver" ); + default: + return NULL; } - - argv[0] = build_path( BINDIR, "wineserver" ); - execv( argv[0], argv ); }
@@ -604,13 +637,15 @@ void start_server( BOOL debug ) if (!started) { int status; + char *pathcopy = NULL; int pid = fork(); if (pid == -1) fatal_error( "fork: %s", strerror(errno) ); if (!pid) { argv[1] = debug ? debug_flag : NULL; argv[2] = NULL; - exec_wineserver( argv ); + while ((argv[0] = next_wineserver_path( &pathcopy ))) + execv( argv[0], argv ); fatal_error( "could not exec wineserver\n" ); } waitpid( pid, &status, 0 );
From: Brendan Shanks bshanks@codeweavers.com
--- dlls/ntdll/unix/loader.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index c184a2d5c4a..b9da6e74fd7 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -30,6 +30,7 @@ #include <stdarg.h> #include <stdio.h> #include <signal.h> +#include <spawn.h> #include <string.h> #include <stdlib.h> #include <sys/types.h> @@ -69,7 +70,6 @@ # include <mach/mach_error.h> # include <mach-o/getsect.h> # include <crt_externs.h> -# include <spawn.h> # ifndef _POSIX_SPAWN_DISABLE_ASLR # define _POSIX_SPAWN_DISABLE_ASLR 0x0100 # endif @@ -77,6 +77,7 @@ #ifdef __ANDROID__ # include <jni.h> #endif +extern char **environ;
#include "ntstatus.h" #define WIN32_NO_STATUS @@ -636,18 +637,23 @@ void start_server( BOOL debug )
if (!started) { - int status; + int status = 1; + pid_t pid; char *pathcopy = NULL; - int pid = fork(); - if (pid == -1) fatal_error( "fork: %s", strerror(errno) ); - if (!pid) + + argv[1] = debug ? debug_flag : NULL; + argv[2] = NULL; + + while ((argv[0] = next_wineserver_path( &pathcopy ))) { - argv[1] = debug ? debug_flag : NULL; - argv[2] = NULL; - while ((argv[0] = next_wineserver_path( &pathcopy ))) - execv( argv[0], argv ); - fatal_error( "could not exec wineserver\n" ); + status = posix_spawn( &pid, argv[0], NULL, NULL, argv, environ ); + free( argv[0] ); + if (!status) + break; } + free( pathcopy ); + if (status) fatal_error( "could not spawn wineserver\n" ); + waitpid( pid, &status, 0 ); status = WIFEXITED(status) ? WEXITSTATUS(status) : 1; if (status == 2) return; /* server lock held by someone else, will retry later */
On Wed Nov 29 01:00:46 2023 +0000, Brendan Shanks wrote:
`fork()` is particularly slow on macOS, this is negligible for a single prefix but adds up for an app like CrossOver which is managing many bottles and starting them to retrieve settings, etc. Also I hadn't considered it, but Chip's point is a good one when Wine is running inside a Mac Cocoa app (like CrossOver's launchers). The error handling and many calls to `posix_spawn()` do complicate the code, I'll see if it would be simpler to pre-generate the possible `wineserver` paths and then loop through them until one succeeds.
I pushed a new version which factors out the generation of possible paths to a function, this allows for just a single call to `execv`/`posix_spawn`.
This merge request was closed by Brendan Shanks.
-
Brendan Shanks -
Brendan Shanks (＠bshanks)