[PATCH 0/5] MR10163: server: Create the new process first thread in new_process request.
In preparation for doing something like https://gitlab.winehq.org/wine/wine/-/merge_requests/10058, this also saves one wineserver request on process creation by creating the first thread in new_process. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10163
From: Rémi Bernon <rbernon@codeweavers.com> --- server/request.c | 2 +- server/thread.c | 12 +++++++----- server/thread.h | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/server/request.c b/server/request.c index 432a5918892..89bba1d9faa 100644 --- a/server/request.c +++ b/server/request.c @@ -563,7 +563,7 @@ static void master_socket_poll_event( struct fd *fd, int event ) fcntl( client, F_SETFL, O_NONBLOCK ); if ((process = create_process( client, NULL, 0, NULL, NULL, NULL, 0, NULL ))) { - create_thread( -1, process, NULL ); + create_thread( -1, process, 0, NULL ); release_object( process ); } } diff --git a/server/thread.c b/server/thread.c index 3aed496450a..ad59361e53b 100644 --- a/server/thread.c +++ b/server/thread.c @@ -501,7 +501,8 @@ static struct context *create_thread_context( struct thread *thread ) /* create a new thread */ -struct thread *create_thread( int fd, struct process *process, const struct security_descriptor *sd ) +struct thread *create_thread( int fd, struct process *process, unsigned int flags, + const struct security_descriptor *sd ) { struct desktop *desktop; struct thread *thread; @@ -578,6 +579,10 @@ struct thread *create_thread( int fd, struct process *process, const struct secu set_fd_events( thread->request_fd, POLLIN ); /* start listening to events */ add_process_thread( thread->process, thread ); + + if (flags & THREAD_CREATE_FLAGS_CREATE_SUSPENDED) thread->suspend++; + thread->dbg_hidden = !!(flags & THREAD_CREATE_FLAGS_HIDE_FROM_DEBUGGER); + thread->bypass_proc_suspend = !!(flags & THREAD_CREATE_FLAGS_BYPASS_PROCESS_FREEZE); return thread; error: @@ -1696,12 +1701,9 @@ DECL_HANDLER(new_thread) goto done; } - if ((thread = create_thread( request_fd, process, sd ))) + if ((thread = create_thread( request_fd, process, req->flags, sd ))) { thread->system_regs = current->system_regs; - if (req->flags & THREAD_CREATE_FLAGS_CREATE_SUSPENDED) thread->suspend++; - thread->dbg_hidden = !!(req->flags & THREAD_CREATE_FLAGS_HIDE_FROM_DEBUGGER); - thread->bypass_proc_suspend = !!(req->flags & THREAD_CREATE_FLAGS_BYPASS_PROCESS_FREEZE); reply->tid = get_thread_id( thread ); if ((reply->handle = alloc_handle_no_access_check( current->process, thread, req->access, objattr->attributes ))) diff --git a/server/thread.h b/server/thread.h index 77ea355483d..317979fc6cd 100644 --- a/server/thread.h +++ b/server/thread.h @@ -105,7 +105,7 @@ extern struct thread *current; /* thread functions */ -extern struct thread *create_thread( int fd, struct process *process, +extern struct thread *create_thread( int fd, struct process *process, unsigned int flags, const struct security_descriptor *sd ); extern struct thread *get_thread_from_id( thread_id_t id ); extern struct thread *get_thread_from_handle( obj_handle_t handle, unsigned int access ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10163
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/ntdll/unix/process.c | 3 ++- server/process.c | 12 +++++++----- server/process.h | 1 + server/protocol.def | 3 ++- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 3a47d5a950a..758a5bf9d35 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -823,7 +823,8 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ req->token = wine_server_obj_handle( token ); req->debug = wine_server_obj_handle( debug ); req->parent_process = wine_server_obj_handle( parent ); - req->flags = process_flags; + req->process_flags = process_flags; + req->thread_flags = thread_flags; req->socket_fd = socketfd[1]; req->access = process_access; req->machine = machine; diff --git a/server/process.c b/server/process.c index b30c835f74f..90328f4b135 100644 --- a/server/process.c +++ b/server/process.c @@ -675,6 +675,7 @@ struct process *create_process( int fd, struct process *parent, unsigned int fla process->unix_pid = -1; process->exit_code = STILL_ACTIVE; process->running_threads = 0; + process->thread_flags = 0; process->priority = PROCESS_PRIOCLASS_NORMAL; process->base_priority = 8; process->disable_boost = 0; @@ -1202,7 +1203,7 @@ DECL_HANDLER(new_process) /* If a job further in the job chain does not permit breakaway process creation * succeeds and the process which is trying to breakaway is assigned to that job. */ - if (parent->job && (req->flags & PROCESS_CREATE_FLAGS_BREAKAWAY) && + if (parent->job && (req->process_flags & PROCESS_CREATE_FLAGS_BREAKAWAY) && !(parent->job->limit_flags & (JOB_OBJECT_LIMIT_BREAKAWAY_OK | JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK))) { set_error( STATUS_ACCESS_DENIED ); @@ -1320,18 +1321,19 @@ DECL_HANDLER(new_process) goto done; } - if (!(process = create_process( socket_fd, parent, req->flags, info->data, sd, + if (!(process = create_process( socket_fd, parent, req->process_flags, info->data, sd, handles, req->handles_size / sizeof(*handles), token ))) goto done; process->machine = req->machine; process->startup_info = (struct startup_info *)grab_object( info ); + process->thread_flags = req->thread_flags; job = parent->job; while (job) { if (!(job->limit_flags & JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK) - && !(req->flags & PROCESS_CREATE_FLAGS_BREAKAWAY + && !(req->process_flags & PROCESS_CREATE_FLAGS_BREAKAWAY && job->limit_flags & JOB_OBJECT_LIMIT_BREAKAWAY_OK)) { add_job_process( job, process ); @@ -1361,7 +1363,7 @@ DECL_HANDLER(new_process) info->data->console = duplicate_handle( parent, info->data->console, process, 0, 0, DUPLICATE_SAME_ACCESS ); - if (!(req->flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES) && info->data->console != 1) + if (!(req->process_flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES) && info->data->console != 1) { info->data->hstdin = duplicate_handle( parent, info->data->hstdin, process, 0, 0, DUPLICATE_SAME_ACCESS | DUPLICATE_SAME_ATTRIBUTES ); @@ -1378,7 +1380,7 @@ DECL_HANDLER(new_process) if (debug_obj) { process->debug_obj = debug_obj; - process->debug_children = !(req->flags & PROCESS_CREATE_FLAGS_NO_DEBUG_INHERIT); + process->debug_children = !(req->process_flags & PROCESS_CREATE_FLAGS_NO_DEBUG_INHERIT); } else if (parent->debug_children) { diff --git a/server/process.h b/server/process.h index 5c136fb5103..5c49daab3f0 100644 --- a/server/process.h +++ b/server/process.h @@ -53,6 +53,7 @@ struct process int unix_pid; /* Unix pid for final SIGKILL */ int exit_code; /* process exit code */ int running_threads; /* number of threads running in this process */ + unsigned int thread_flags; /* first thread flags */ timeout_t start_time; /* absolute time at process start */ timeout_t end_time; /* absolute time at process end */ affinity_t affinity; /* process affinity mask */ diff --git a/server/protocol.def b/server/protocol.def index 0ac64297026..ff995f26987 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1087,7 +1087,8 @@ typedef volatile struct obj_handle_t token; /* process token */ obj_handle_t debug; /* process debug object */ obj_handle_t parent_process; /* parent process */ - unsigned int flags; /* process creation flags */ + unsigned int process_flags; /* process creation flags */ + unsigned int thread_flags; /* main thread creation flags */ int socket_fd; /* file descriptor for process socket */ unsigned int access; /* access rights for process object */ unsigned short machine; /* architecture that the new process will use */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10163
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/ntdll/unix/process.c | 12 ++++++++++-- server/process.c | 23 ++++++++++++++++++++++- server/process.h | 1 + server/protocol.def | 2 ++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 758a5bf9d35..005b90eb05e 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -690,8 +690,8 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ unsigned int status; BOOL success = FALSE; HANDLE file_handle, process_info = 0, process_handle = 0, thread_handle = 0; - struct object_attributes *objattr; - data_size_t attr_len; + struct object_attributes *objattr, *thread_objattr; + data_size_t attr_len, thread_attr_len; char *winedebug = NULL; char *unix_name = NULL; struct startup_info_data *startup_info = NULL; @@ -782,6 +782,11 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ env_size = get_env_size( params, &winedebug ); if ((status = alloc_object_attributes( process_attr, &objattr, &attr_len ))) goto done; + if ((status = alloc_object_attributes( thread_attr, &thread_objattr, &thread_attr_len ))) + { + free( thread_objattr ); + goto done; + } if ((status = alloc_handle_list( handles_attr, &handles, &handles_size ))) { @@ -831,9 +836,11 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ req->info_size = startup_info_size; req->handles_size = handles_size; req->jobs_size = jobs_size; + req->sd_len = thread_objattr ? thread_objattr->sd_len : 0; wine_server_add_data( req, objattr, attr_len ); wine_server_add_data( req, handles, handles_size ); wine_server_add_data( req, jobs, jobs_size ); + wine_server_add_data( req, thread_objattr + 1, req->sd_len ); wine_server_add_data( req, startup_info, startup_info_size ); wine_server_add_data( req, params->Environment, env_size ); if (!(status = wine_server_call( req ))) @@ -845,6 +852,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ } SERVER_END_REQ; close( socketfd[1] ); + free( thread_objattr ); free( objattr ); free( handles ); free( jobs ); diff --git a/server/process.c b/server/process.c index 90328f4b135..5e44e6faaa6 100644 --- a/server/process.c +++ b/server/process.c @@ -676,6 +676,7 @@ struct process *create_process( int fd, struct process *parent, unsigned int fla process->exit_code = STILL_ACTIVE; process->running_threads = 0; process->thread_flags = 0; + process->thread_sd = NULL; process->priority = PROCESS_PRIOCLASS_NORMAL; process->base_priority = 8; process->disable_boost = 0; @@ -802,6 +803,7 @@ static void process_destroy( struct object *obj ) free( process->rawinput_devices ); free( process->dir_cache ); free( process->image ); + free( process->thread_sd ); } /* dump a process on stdout for debugging purposes */ @@ -1153,7 +1155,7 @@ DECL_HANDLER(new_process) struct startup_info *info; const void *info_ptr; struct unicode_str name, desktop_path = {0}; - const struct security_descriptor *sd; + const struct security_descriptor *sd, *thread_sd = NULL; const struct object_attributes *objattr = get_req_object_attributes( &sd, &name, NULL ); struct process *process = NULL; struct token *token = NULL; @@ -1256,6 +1258,24 @@ DECL_HANDLER(new_process) info_ptr = (const char *)info_ptr + req->jobs_size; info->data_size -= req->jobs_size; } + if (req->sd_len > info->data_size) + { + set_error( STATUS_INVALID_PARAMETER ); + close( socket_fd ); + goto done; + } + if (req->sd_len) + { + thread_sd = info_ptr; + info_ptr = (const char *)thread_sd + req->sd_len; + info->data_size -= req->sd_len; + } + if (thread_sd && !sd_is_valid( thread_sd, req->sd_len )) + { + set_error( STATUS_INVALID_PARAMETER ); + close( socket_fd ); + goto done; + } job_handle_count = req->jobs_size / sizeof(*handles); for (i = 0; i < job_handle_count; ++i) @@ -1328,6 +1348,7 @@ DECL_HANDLER(new_process) process->machine = req->machine; process->startup_info = (struct startup_info *)grab_object( info ); process->thread_flags = req->thread_flags; + if (thread_sd && !(process->thread_sd = memdup( thread_sd, req->sd_len ))) goto done; job = parent->job; while (job) diff --git a/server/process.h b/server/process.h index 5c49daab3f0..b2292440c8b 100644 --- a/server/process.h +++ b/server/process.h @@ -54,6 +54,7 @@ struct process int exit_code; /* process exit code */ int running_threads; /* number of threads running in this process */ unsigned int thread_flags; /* first thread flags */ + struct security_descriptor *thread_sd; /* first thread security descriptor */ timeout_t start_time; /* absolute time at process start */ timeout_t end_time; /* absolute time at process end */ affinity_t affinity; /* process affinity mask */ diff --git a/server/protocol.def b/server/protocol.def index ff995f26987..3480165aacd 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1095,9 +1095,11 @@ typedef volatile struct data_size_t info_size; /* size of startup info */ data_size_t handles_size; /* length of explicit handles list */ data_size_t jobs_size; /* length of jobs list */ + data_size_t sd_len; /* size of security descriptor */ VARARG(objattr,object_attributes); /* object attributes */ VARARG(handles,uints,handles_size); /* handles list */ VARARG(jobs,uints,jobs_size); /* jobs list */ + VARARG(sd,security_descriptor,sd_len); /* thread security descriptor */ VARARG(info,startup_info,info_size); /* startup information */ VARARG(env,unicode_str); /* environment for new process */ @REPLY -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10163
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/ntdll/unix/process.c | 13 ++++++------- server/process.c | 18 +++++++++++++----- server/protocol.def | 4 ++++ server/thread.c | 1 + 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 005b90eb05e..1435d6160eb 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -877,15 +877,10 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ SERVER_START_REQ( new_thread ) { req->process = wine_server_obj_handle( process_handle ); - req->access = thread_access; req->flags = thread_flags; req->request_fd = -1; wine_server_add_data( req, objattr, attr_len ); - if (!(status = wine_server_call( req ))) - { - thread_handle = wine_server_ptr_handle( reply->handle ); - id.UniqueThread = ULongToHandle( reply->tid ); - } + status = wine_server_call( req ); } SERVER_END_REQ; free( objattr ); @@ -903,10 +898,14 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ NtWaitForSingleObject( process_info, FALSE, NULL ); SERVER_START_REQ( get_new_process_info ) { - req->info = wine_server_obj_handle( process_info ); + req->info = wine_server_obj_handle( process_info ); + req->access = thread_access; + req->attributes = thread_attr ? thread_attr->Attributes : 0; wine_server_call( req ); success = reply->success; status = reply->exit_code; + thread_handle = wine_server_ptr_handle( reply->handle ); + id.UniqueThread = ULongToHandle( reply->tid ); } SERVER_END_REQ; diff --git a/server/process.c b/server/process.c index 5e44e6faaa6..78727080e9d 100644 --- a/server/process.c +++ b/server/process.c @@ -1431,14 +1431,22 @@ DECL_HANDLER(new_process) DECL_HANDLER(get_new_process_info) { struct startup_info *info; + struct thread *thread; - if ((info = (struct startup_info *)get_handle_obj( current->process, req->info, - 0, &startup_info_ops ))) + if (!(info = (struct startup_info *)get_handle_obj( current->process, req->info, 0, &startup_info_ops ))) return; + if (!(thread = get_process_first_thread( info->process ))) { - reply->success = is_process_init_done( info->process ); - reply->exit_code = info->process->exit_code; - release_object( info ); + set_error( STATUS_INVALID_PARAMETER ); + goto done; } + + reply->tid = get_thread_id( thread ); + reply->handle = alloc_handle_no_access_check( current->process, thread, req->access, req->attributes ); + reply->success = is_process_init_done( info->process ); + reply->exit_code = info->process->exit_code; + +done: + release_object( info ); } /* Itererate processes using global process list */ diff --git a/server/protocol.def b/server/protocol.def index 3480165aacd..78e2042f8e6 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1112,7 +1112,11 @@ typedef volatile struct /* Retrieve information about a newly started process */ @REQ(get_new_process_info) obj_handle_t info; /* info handle returned from new_process_request */ + unsigned int access; /* wanted handle access rights */ + unsigned int attributes; /* handle object attributes */ @REPLY + thread_id_t tid; /* main thread id */ + obj_handle_t handle; /* main thread handle (in the current process) */ int success; /* did the process start successfully? */ int exit_code; /* process exit code if failed */ @END diff --git a/server/thread.c b/server/thread.c index ad59361e53b..472fd85790a 100644 --- a/server/thread.c +++ b/server/thread.c @@ -1705,6 +1705,7 @@ DECL_HANDLER(new_thread) { thread->system_regs = current->system_regs; reply->tid = get_thread_id( thread ); + if (request_fd == -1) goto done; /* thread handle will be returned from get_new_process_info */ if ((reply->handle = alloc_handle_no_access_check( current->process, thread, req->access, objattr->attributes ))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10163
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/ntdll/unix/process.c | 14 -------------- server/process.c | 6 +++++- server/thread.c | 15 +++------------ 3 files changed, 8 insertions(+), 27 deletions(-) diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 1435d6160eb..e274fc66f17 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -872,20 +872,6 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ goto done; } - if ((status = alloc_object_attributes( thread_attr, &objattr, &attr_len ))) goto done; - - SERVER_START_REQ( new_thread ) - { - req->process = wine_server_obj_handle( process_handle ); - req->flags = thread_flags; - req->request_fd = -1; - wine_server_add_data( req, objattr, attr_len ); - status = wine_server_call( req ); - } - SERVER_END_REQ; - free( objattr ); - if (status) goto done; - /* create the child process */ if ((status = spawn_process( params, socketfd[0], unixdir, winedebug, &pe_info ))) goto done; diff --git a/server/process.c b/server/process.c index 78727080e9d..4c778d800db 100644 --- a/server/process.c +++ b/server/process.c @@ -1161,7 +1161,7 @@ DECL_HANDLER(new_process) struct token *token = NULL; struct debug_obj *debug_obj = NULL; struct process *parent; - struct thread *parent_thread = current; + struct thread *thread, *parent_thread = current; int socket_fd = thread_get_inflight_fd( current, req->socket_fd ); const obj_handle_t *handles = NULL; const obj_handle_t *job_handles = NULL; @@ -1415,6 +1415,10 @@ DECL_HANDLER(new_process) info->data->process_group_id = process->group_id; info->process = (struct process *)grab_object( process ); + + if (!(thread = create_thread( -1, process, process->thread_flags, process->thread_sd ))) goto done; + thread->system_regs = current->system_regs; + reply->info = alloc_handle( current->process, info, SYNCHRONIZE, 0 ); reply->pid = get_process_id( process ); reply->handle = alloc_handle_no_access_check( current->process, process, req->access, objattr->attributes ); diff --git a/server/thread.c b/server/thread.c index 472fd85790a..54b1b5e2c00 100644 --- a/server/thread.c +++ b/server/thread.c @@ -1676,17 +1676,9 @@ DECL_HANDLER(new_thread) if (process != current->process) { - if (request_fd != -1) /* can't create a request fd in a different process */ - { - close( request_fd ); - set_error( STATUS_INVALID_PARAMETER ); - goto done; - } - if (process->running_threads) /* only the initial thread can be created in another process */ - { - set_error( STATUS_ACCESS_DENIED ); - goto done; - } + if (request_fd != -1) close( request_fd ); + set_error( STATUS_ACCESS_DENIED ); + goto done; } else if (request_fd == -1 || fcntl( request_fd, F_SETFL, O_NONBLOCK ) == -1) { @@ -1705,7 +1697,6 @@ DECL_HANDLER(new_thread) { thread->system_regs = current->system_regs; reply->tid = get_thread_id( thread ); - if (request_fd == -1) goto done; /* thread handle will be returned from get_new_process_info */ if ((reply->handle = alloc_handle_no_access_check( current->process, thread, req->access, objattr->attributes ))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10163
We used to do that, but it was split in 39afcaac4a7a713188c5c23f5fe5114fc360c8d4 to avoid having to cram everything into a single request. I don't think we want to change it back (and TBH I'm still not convinced that we want !10058). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10163#note_130200
On Mon Feb 23 11:31:37 2026 +0000, Alexandre Julliard wrote:
We used to do that, but it was split in 39afcaac4a7a713188c5c23f5fe5114fc360c8d4 to avoid having to cram everything into a single request. I don't think we want to change it back (and TBH I'm still not convinced that we want !10058). Okay, well I'm not fully convinced either, and actually the more I look into it the more I am convinced that host GUI integration belongs to a separate process, wineserver in particular as pretty much all of it needs to go through it eventually. When I suggested that idea for X11 (but now I'm starting to think that all platforms could do the same) you didn't seem very excited about it either ;). Of course, it could be another process (lets say dwm.exe), but that seems more inefficient and unnecessarily complicated to me.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10163#note_130206
participants (3)
-
Alexandre Julliard (＠julliard) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)