[PATCH] server: Track desktop users per thread instead of per process.
Rémi Bernon
rbernon at codeweavers.com
Mon Apr 19 03:36:41 CDT 2021
On 4/19/21 10:21 AM, Rémi Bernon wrote:
> As some thread may use a different desktop from their process.
>
> This fixes the user32 win tests, which leaks a desktop that never gets
> closed. The test_shell_window test creates a new desktop, which spawns
> explorer.exe process, incrementing the desktop user count to 1, then
> associates the desktop to a thread, which closes it on exit.
>
> Never the user count is incremented to 2, and closing the thread desktop
> doesn't either check whether the desktop process should be terminated.
>
> Reversely, it is possible to create a desktop, associate it with a
> thread /and/ a process, and this time the desktop process would be
> terminated when the process exits, although the thread may still be
> using it.
>
> Tracking the users per thread is more robust and fixes the problem as
> set_thread_desktop increments the desktop user count, and thread exit
> decrements it.
>
> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
> ---
> server/process.c | 3 +++
> server/thread.c | 10 +++++++-
> server/user.h | 2 ++
> server/winstation.c | 62 +++++++++++++++++++++++++++------------------
> 4 files changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/server/process.c b/server/process.c
> index 8ad5a59a20b..9a3da5672a0 100644
> --- a/server/process.c
> +++ b/server/process.c
> @@ -1503,6 +1503,7 @@ DECL_HANDLER(get_process_idle_event)
> DECL_HANDLER(make_process_system)
> {
> struct process *process = current->process;
> + struct thread *thread;
>
> if (!shutdown_event)
> {
> @@ -1516,6 +1517,8 @@ DECL_HANDLER(make_process_system)
> if (!process->is_system)
> {
> process->is_system = 1;
> + LIST_FOR_EACH_ENTRY( thread, &process->thread_list, struct thread, proc_entry )
> + close_thread_desktop( thread );
> close_process_desktop( process );
> if (!--user_processes && !shutdown_stage && master_socket_timeout != TIMEOUT_INFINITE)
> shutdown_timeout = add_timeout_user( master_socket_timeout, server_shutdown_timeout, NULL );
> diff --git a/server/thread.c b/server/thread.c
> index fe9f9bdec37..5375b055758 100644
> --- a/server/thread.c
> +++ b/server/thread.c
> @@ -306,6 +306,7 @@ 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 desktop *desktop;
> struct thread *thread;
> int request_pipe[2];
>
> @@ -342,7 +343,7 @@ struct thread *create_thread( int fd, struct process *process, const struct secu
> init_thread_structure( thread );
>
> thread->process = (struct process *)grab_object( process );
> - thread->desktop = process->desktop;
> + thread->desktop = 0;
> thread->affinity = process->affinity;
> if (!current) current = thread;
>
> @@ -369,6 +370,13 @@ struct thread *create_thread( int fd, struct process *process, const struct secu
> return NULL;
> }
>
> + if (process->desktop && (desktop = get_desktop_obj( process, process->desktop, 0 )))
> + {
> + set_thread_default_desktop( process, thread, desktop, process->desktop );
> + release_object( desktop );
> + }
> + clear_error(); /* ignore errors */
> +
> set_fd_events( thread->request_fd, POLLIN ); /* start listening to events */
> add_process_thread( thread->process, thread );
> return thread;
> diff --git a/server/user.h b/server/user.h
> index 6267f3e2881..a5f906cea51 100644
> --- a/server/user.h
> +++ b/server/user.h
> @@ -185,6 +185,8 @@ extern struct winstation *get_process_winstation( struct process *process, unsig
> extern struct desktop *get_thread_desktop( struct thread *thread, unsigned int access );
> extern void connect_process_winstation( struct process *process, struct thread *parent_thread,
> struct process *parent_process );
> +extern void set_thread_default_desktop( struct process *process, struct thread *thread,
> + struct desktop *desktop, obj_handle_t handle );
> extern void set_process_default_desktop( struct process *process, struct desktop *desktop,
> obj_handle_t handle );
> extern void close_process_desktop( struct process *process );
> diff --git a/server/winstation.c b/server/winstation.c
> index 1c7552f0687..94527899311 100644
> --- a/server/winstation.c
> +++ b/server/winstation.c
> @@ -324,15 +324,23 @@ static void add_desktop_user( struct desktop *desktop )
> /* remove a user of the desktop and start the close timeout if necessary */
> static void remove_desktop_user( struct desktop *desktop )
> {
> + struct process *process;
> assert( desktop->users > 0 );
> desktop->users--;
>
> /* if we have one remaining user, it has to be the manager of the desktop window */
> - if (desktop->users == 1 && get_top_window_owner( desktop ))
> - {
> - assert( !desktop->close_timeout );
> + if ((process = get_top_window_owner( desktop )) && desktop->users == process->running_threads && !desktop->close_timeout)
> desktop->close_timeout = add_timeout_user( -TICKS_PER_SEC, close_desktop_timeout, desktop );
> - }
> +}
> +
> +/* set the thread desktop to the process default desktop */
> +void set_thread_default_desktop( struct process *process, struct thread *thread,
> + struct desktop *desktop, obj_handle_t handle )
> +{
> + if (thread->desktop || thread->desktop == handle) return; /* nothing to do */
> + thread->desktop = handle;
> +
> + if (!process->is_system) add_desktop_user( desktop );
> }
>
> /* set the process default desktop handle */
> @@ -340,24 +348,13 @@ void set_process_default_desktop( struct process *process, struct desktop *deskt
> obj_handle_t handle )
> {
> struct thread *thread;
> - struct desktop *old_desktop;
>
> if (process->desktop == handle) return; /* nothing to do */
> -
> - if (!(old_desktop = get_desktop_obj( process, process->desktop, 0 ))) clear_error();
> process->desktop = handle;
>
> /* set desktop for threads that don't have one yet */
> LIST_FOR_EACH_ENTRY( thread, &process->thread_list, struct thread, proc_entry )
> - if (!thread->desktop) thread->desktop = handle;
> -
> - if (!process->is_system && desktop != old_desktop)
> - {
> - add_desktop_user( desktop );
> - if (old_desktop) remove_desktop_user( old_desktop );
> - }
> -
> - if (old_desktop) release_object( old_desktop );
> + set_thread_default_desktop( process, thread, desktop, handle );
> }
>
> /* connect a process to its window station */
> @@ -413,23 +410,31 @@ done:
> /* close the desktop of a given process */
> void close_process_desktop( struct process *process )
> {
> - struct desktop *desktop;
> + obj_handle_t handle;
>
> - if (process->desktop && (desktop = get_desktop_obj( process, process->desktop, 0 )))
> - {
> - remove_desktop_user( desktop );
> - release_object( desktop );
> - }
> - clear_error(); /* ignore errors */
> + if (!(handle = process->desktop)) return;
> + process->desktop = 0;
> +
> + close_handle( process, handle );
> }
>
> /* close the desktop of a given thread */
> void close_thread_desktop( struct thread *thread )
> {
> - obj_handle_t handle = thread->desktop;
> + struct desktop *desktop;
> + obj_handle_t handle;
>
> + if (!(handle = thread->desktop)) return;
> thread->desktop = 0;
> - if (handle) close_handle( thread->process, handle );
> +
> + if (!thread->process->is_system && (desktop = get_desktop_obj( thread->process, handle, 0 )))
> + {
> + remove_desktop_user( desktop );
> + release_object( desktop );
> + }
> + clear_error(); /* ignore errors */
> +
> + close_handle( thread->process, handle );
> }
>
> /* create a window station */
> @@ -624,7 +629,14 @@ DECL_HANDLER(set_thread_desktop)
> if (old_desktop != new_desktop && current->desktop_users > 0)
> set_error( STATUS_DEVICE_BUSY );
> else
> + {
> current->desktop = req->handle; /* FIXME: should we close the old one? */
> + if (!current->process->is_system && old_desktop != new_desktop)
> + {
> + add_desktop_user( new_desktop );
> + if (old_desktop) remove_desktop_user( old_desktop );
> + }
> + }
>
> if (!current->process->desktop)
> set_process_default_desktop( current->process, new_desktop, req->handle );
>
Please, ignore this version I just messed everything in a last minute
cleanup.
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list