[PATCH 0/2] MR9823: server: Track foreground process id, instead of looking for active window.
When foreground input is changed and given to a different process (for instance the desktop process), that process active window is not set until it handles its internal messages. This might cause a race condition between the new activated process and the original process trying to steal foreground back. We might allow it to get foreground back if we don't find any active window at the time of the set_foreground_window call. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58167 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9823
From: Rémi Bernon <rbernon@codeweavers.com> When foreground input is changed and given to a different process (for instance the desktop process), that process active window is not set until it handles its internal messages. This might cause a race condition between the new activated process and the original process trying to steal foreground back. We might allow it to get foreground back if we don't find any active window at the time of the set_foreground_window call. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58167 --- server/queue.c | 21 ++++++++++----------- server/user.h | 1 + server/winstation.c | 1 + 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/server/queue.c b/server/queue.c index 3e8ab2ef48a..4c3673fc1c4 100644 --- a/server/queue.c +++ b/server/queue.c @@ -662,7 +662,7 @@ void set_clip_rectangle( struct desktop *desktop, const struct rectangle *rect, } /* change the foreground input and reset the cursor clip rect */ -static void set_foreground_input( struct desktop *desktop, struct thread_input *input ) +static void set_foreground_input( struct desktop *desktop, struct process *process, struct thread_input *input ) { input_shm_t *input_shm, *old_input_shm; shared_object_t dummy_obj = {0}; @@ -674,6 +674,7 @@ static void set_foreground_input( struct desktop *desktop, struct thread_input * set_clip_rectangle( desktop, NULL, SET_CURSOR_NOCLIP, 1 ); desktop->foreground_input = input; + desktop->foreground_pid = process->id; SHARED_WRITE_BEGIN( old_input_shm, input_shm_t ) { @@ -1374,7 +1375,11 @@ static void thread_input_destroy( struct object *obj ) empty_msg_list( &input->msg_list ); if ((desktop = input->desktop)) { - if (desktop->foreground_input == input) desktop->foreground_input = NULL; + if (desktop->foreground_input == input) + { + desktop->foreground_input = NULL; + desktop->foreground_pid = 0; + } release_object( desktop ); } if (input->shared) free_shared_object( input->shared ); @@ -2007,14 +2012,8 @@ static struct thread *get_foreground_thread( struct desktop *desktop, user_handl static int is_current_process_foreground( struct desktop *desktop ) { - struct thread *thread; - int ret; - - if (!(thread = get_foreground_thread( desktop, 0 ))) return 1; - ret = thread->process == current->process || thread->process->id == current->process->parent_id; - release_object( thread ); - - return ret; + return !desktop->foreground_pid || desktop->foreground_pid == current->process->id || + desktop->foreground_pid == current->process->parent_id; } /* user32 reserves 1 & 2 for winemouse and winekeyboard, @@ -3841,7 +3840,7 @@ DECL_HANDLER(set_foreground_window) reply->previous = desktop->foreground_input ? desktop->foreground_input->shared->active : 0; reply->send_msg_old = (reply->previous && desktop->foreground_input != queue->input); - set_foreground_input( desktop, req->internal || !is_desktop ? thread->queue->input : NULL ); + set_foreground_input( desktop, thread->process, req->internal || !is_desktop ? thread->queue->input : NULL ); reply->send_msg_new = (desktop->foreground_input != queue->input); done: diff --git a/server/user.h b/server/user.h index 9f6c3dcaf0f..060e54ec775 100644 --- a/server/user.h +++ b/server/user.h @@ -79,6 +79,7 @@ struct desktop struct list pointers; /* list of active pointers */ struct timeout_user *close_timeout; /* timeout before closing the desktop */ struct thread_input *foreground_input; /* thread input of foreground thread */ + process_id_t foreground_pid; /* id of the foreground process */ unsigned int users; /* processes and threads using this desktop */ unsigned char keystate[256]; /* asynchronous key state */ unsigned char alt_pressed; /* last key press was Alt (used to determine msg on release) */ diff --git a/server/winstation.c b/server/winstation.c index a36253e20db..8820d0b6bcd 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -298,6 +298,7 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned desktop->global_hooks = NULL; desktop->close_timeout = NULL; desktop->foreground_input = NULL; + desktop->foreground_pid = 0; desktop->users = 0; list_init( &desktop->threads ); desktop->clip_flags = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9823
From: Rémi Bernon <rbernon@codeweavers.com> We allow windows to be each activated at least once after their creation and this is now mostly unnecessary. It also effectively allows processes to activate their first window twice, once because the window was never active before, and a second time because we only set the process-wide flag after checking the window flag, which is not the intended effect. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58167 --- server/process.c | 1 - server/process.h | 1 - server/queue.c | 6 +++--- server/winstation.c | 1 - 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/server/process.c b/server/process.c index a06aa83166c..b30c835f74f 100644 --- a/server/process.c +++ b/server/process.c @@ -682,7 +682,6 @@ struct process *create_process( int fd, struct process *parent, unsigned int fla process->is_system = 0; process->debug_children = 1; process->is_terminating = 0; - process->set_foreground = 0; process->imagelen = 0; process->image = NULL; process->job = NULL; diff --git a/server/process.h b/server/process.h index 7cfcc39c258..5c136fb5103 100644 --- a/server/process.h +++ b/server/process.h @@ -63,7 +63,6 @@ struct process unsigned int is_system:1; /* is it a system process? */ unsigned int debug_children:1;/* also debug all child processes */ unsigned int is_terminating:1;/* is process terminating? */ - unsigned int set_foreground:1;/* has process called set_foreground_window */ data_size_t imagelen; /* length of image path in bytes */ WCHAR *image; /* main exe image full path */ struct job *job; /* job object associated with this process */ diff --git a/server/queue.c b/server/queue.c index 4c3673fc1c4..b7c5adb029c 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3829,9 +3829,9 @@ DECL_HANDLER(set_foreground_window) if (set_foreground && !req->internal) { - if (!current->process->set_foreground) current->process->set_foreground = 1; - else if (!is_current_process_foreground( desktop ) && queue->input && desktop->foreground_input && - queue->input->user_time < desktop->foreground_input->user_time) + /* allow a process to set foreground after changing desktop, or each window to be set foreground at least once */ + if (!is_current_process_foreground( desktop ) && queue->input && desktop->foreground_input && + queue->input->user_time < desktop->foreground_input->user_time) { set_error( STATUS_ACCESS_DENIED ); goto done; diff --git a/server/winstation.c b/server/winstation.c index 8820d0b6bcd..90e55b80d7d 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -865,7 +865,6 @@ DECL_HANDLER(set_thread_desktop) { if (old_desktop) remove_desktop_thread( old_desktop, current ); add_desktop_thread( new_desktop, current ); - current->process->set_foreground = 0; } reply->locator = get_shared_object_locator( new_desktop->shared ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9823
participants (1)
-
Rémi Bernon