[PATCH v3] server: Track desktop users per thread instead of per process.

[Rémi Bernon]

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(a)codeweavers.com>
---

v3: Rename close_thread_desktop to release_thread_desktop, and add a
    close argument to only close and clear the handle on thread exit.

    When making a process system, only release the desktop user count,
    and don't touch the threads or the process desktop handles.

    Small cleanup on the clear_error() calls too, to only do them when
    get_desktop_obj failed.

 server/process.c    |  6 ++--
 server/thread.c     | 15 ++++++++--
 server/user.h       |  3 +-
 server/winstation.c | 72 ++++++++++++++++++++++++++++-----------------
 4 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/server/process.c b/server/process.c
index 8ad5a59a20b..d819f03a615 100644
--- a/server/process.c
+++ b/server/process.c
@@ -841,7 +841,7 @@ static void process_killed( struct process *process )
 
     assert( list_empty( &process->thread_list ));
     process->end_time = current_time;
-    if (!process->is_system) close_process_desktop( process );
+    close_process_desktop( process );
     process->winstation = 0;
     process->desktop = 0;
     cancel_process_asyncs( process );
@@ -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)
     {
@@ -1515,8 +1516,9 @@ DECL_HANDLER(make_process_system)
 
     if (!process->is_system)
     {
+        LIST_FOR_EACH_ENTRY( thread, &process->thread_list, struct thread, proc_entry )
+            release_thread_desktop( thread, 0 );
         process->is_system = 1;
-        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..66f1057d1db 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,16 @@ struct thread *create_thread( int fd, struct process *process, const struct secu
         return NULL;
     }
 
+    if (process->desktop)
+    {
+        if (!(desktop = get_desktop_obj( process, process->desktop, 0 ))) clear_error();  /* ignore errors */
+        else
+        {
+            set_thread_default_desktop( thread, desktop, process->desktop );
+            release_object( desktop );
+        }
+    }
+
     set_fd_events( thread->request_fd, POLLIN );  /* start listening to events */
     add_process_thread( thread->process, thread );
     return thread;
@@ -416,7 +427,7 @@ static void cleanup_thread( struct thread *thread )
     cleanup_clipboard_thread(thread);
     destroy_thread_windows( thread );
     free_msg_queue( thread );
-    close_thread_desktop( thread );
+    release_thread_desktop( thread, 1 );
     for (i = 0; i < MAX_INFLIGHT_FDS; i++)
     {
         if (thread->inflight[i].client != -1)
diff --git a/server/user.h b/server/user.h
index 6267f3e2881..80f7e91f12c 100644
--- a/server/user.h
+++ b/server/user.h
@@ -188,7 +188,8 @@ extern void connect_process_winstation( struct process *process, struct thread *
 extern void set_process_default_desktop( struct process *process, struct desktop *desktop,
                                          obj_handle_t handle );
 extern void close_process_desktop( struct process *process );
-extern void close_thread_desktop( struct thread *thread );
+extern void set_thread_default_desktop( struct thread *thread, struct desktop *desktop, obj_handle_t handle );
+extern void release_thread_desktop( struct thread *thread, int close );
 
 static inline int is_rect_empty( const rectangle_t *rect )
 {
diff --git a/server/winstation.c b/server/winstation.c
index 1c7552f0687..61f9f77c73f 100644
--- a/server/winstation.c
+++ b/server/winstation.c
@@ -324,15 +324,22 @@ 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 default desktop handle */
+void set_thread_default_desktop( struct thread *thread, struct desktop *desktop, obj_handle_t handle )
+{
+    if (thread->desktop) return;  /* nothing to do */
+
+    thread->desktop = handle;
+    if (!thread->process->is_system) add_desktop_user( desktop );
 }
 
 /* set the process default desktop handle */
@@ -340,24 +347,14 @@ 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( thread, desktop, handle );
 }
 
 /* connect a process to its window station */
@@ -413,23 +410,37 @@ 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 )
+/* release (and eventually close) the desktop of a given thread */
+void release_thread_desktop( struct thread *thread, int close )
 {
-    obj_handle_t handle = thread->desktop;
+    struct desktop *desktop;
+    obj_handle_t handle;
+
+    if (!(handle = thread->desktop)) return;
+
+    if (!thread->process->is_system)
+    {
+        if (!(desktop = get_desktop_obj( thread->process, handle, 0 ))) clear_error();  /* ignore errors */
+        else
+        {
+            remove_desktop_user( desktop );
+            release_object( desktop );
+        }
+    }
 
-    thread->desktop = 0;
-    if (handle) close_handle( thread->process, handle );
+    if (close)
+    {
+        thread->desktop = 0;
+        close_handle( thread->process, handle );
+    }
 }
 
 /* create a window station */
@@ -624,7 +635,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 );
-- 
2.31.0