[PATCH 0/1] MR7039: server: Destroy global hook table in desktop_close_handle().

[Paul Gofman (＠gofman)]

Commit f784374bcb5195c918754b1da6142a618a0da7dd introduced a cyclic reference between desktop and global hook through global hook table. So now if a hook was set and then not explicitly unset and the process was just terminated the hook, desktop, thread and some other related objects are leaked (which is displayed by wineserver on prefix shutdown in this case).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7039

[PATCH 1/1] server: Destroy global hook table in desktop_close_handle().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 server/winstation.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/server/winstation.c b/server/winstation.c
index b3746090ccf..6c243f809c1 100644
--- a/server/winstation.c
+++ b/server/winstation.c
@@ -369,12 +369,19 @@ static int desktop_link_name( struct object *obj, struct object_name *name, stru
 
 static int desktop_close_handle( struct object *obj, struct process *process, obj_handle_t handle )
 {
+    struct desktop *desktop = (struct desktop *)obj;
     struct thread *thread;
 
     /* check if the handle is currently used by the process or one of its threads */
     if (process->desktop == handle) return 0;
     LIST_FOR_EACH_ENTRY( thread, &process->thread_list, struct thread, proc_entry )
         if (thread->desktop == handle) return 0;
+
+    if (desktop->obj.handle_count == 1 && desktop->global_hooks)
+    {
+        release_object( desktop->global_hooks );
+        desktop->global_hooks = NULL;
+    }
     return 1;
 }
 
@@ -397,7 +404,6 @@ static void desktop_destroy( struct object *obj )
     free_pointers( desktop );
     if (desktop->top_window) free_window_handle( desktop->top_window );
     if (desktop->msg_window) free_window_handle( desktop->msg_window );
-    if (desktop->global_hooks) release_object( desktop->global_hooks );
     if (desktop->close_timeout) remove_timeout_user( desktop->close_timeout );
     if (desktop->key_repeat.timeout) remove_timeout_user( desktop->key_repeat.timeout );
     release_object( desktop->winstation );
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7039

[Rémi Bernon]

Isn't `remove_thread_hooks` called from `free_msg_queue` supposed to cleanup any hooks on thread exit?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7039#note_90786

[Paul Gofman (＠gofman)]

The attached test program is enough to see the issue without the patch (in wineserver output on shutdown listing leaked objects), so the issue is there. I will look why that doesn't happen on thread cleanup though, maybe it should be fixed there indeed.
[hookclean.c](/uploads/8a8da3879c58b0d691b4c1ebf90638b4/hookclean.c)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7039#note_90794