From d05ac476a0ca682ecb7ae13aa9e578c94b71e3f8 Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Mon, 12 Nov 2018 18:10:32 +0200
Subject: [PATCH] server: Do not signal violently terminated threads until they
 are really gone
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a thread is terminated violently (such as by using TerminateThread)
that is not the current thread, the server sends a signal to the thread to
terminate it, but it immediately wakes up anything waiting on it. The caller
can expect WaitForSingleObject (or similar) to return when the thread is
really gone and doesn't execute anything anymore, and this is exactly what
happens on Windows.

If that thread was altering global state, and the thread that was waiting
on it will read (or alter) the global state *after* waiting for it and
expecting it to not change (because it assumes the thread is terminated by
that point, as on Windows), the result will be a race condition, since there's
no guarantee currently that the terminated thread really stopped executing.

Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com>
---
 server/thread.c | 32 +++++++++++++++++++++++++++++---
 server/thread.h |  1 +
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/server/thread.c b/server/thread.c
index 9b14174578e..2f32cb4b62e 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -239,6 +239,7 @@ static inline void init_thread_structure( struct thread *thread )
     thread->token           = NULL;
     thread->desc            = NULL;
     thread->desc_len        = 0;
+    thread->exit_poll       = NULL;
 
     thread->creation_time = current_time;
     thread->exit_time     = 0;
@@ -430,6 +431,7 @@ static void destroy_thread( struct object *obj )
     list_remove( &thread->entry );
     cleanup_thread( thread );
     release_object( thread->process );
+    if (thread->exit_poll) remove_timeout_user( thread->exit_poll );
     if (thread->id) free_ptid( thread->id );
     if (thread->token) release_object( thread->token );
 }
@@ -454,7 +456,7 @@ static struct object_type *thread_get_type( struct object *obj )
 static int thread_signaled( struct object *obj, struct wait_queue_entry *entry )
 {
     struct thread *mythread = (struct thread *)obj;
-    return (mythread->state == TERMINATED);
+    return mythread->state == TERMINATED && !mythread->exit_poll;
 }
 
 static unsigned int thread_map_access( struct object *obj, unsigned int access )
@@ -1245,6 +1247,26 @@ int thread_get_inflight_fd( struct thread *thread, int client )
     return -1;
 }
 
+static void check_terminated( void *arg )
+{
+    struct thread *thread = arg;
+    assert( thread->obj.ops == &thread_ops );
+    assert( thread->state == TERMINATED );
+
+    /* don't wake up until the thread is really dead, to avoid race conditions */
+    if (thread->unix_tid != -1 && !kill( thread->unix_tid, 0 ))
+    {
+        thread->exit_poll = add_timeout_user( -TICKS_PER_SEC / 1000, check_terminated, thread );
+        return;
+    }
+
+    /* grab reference since object can be destroyed while trying to wake up */
+    grab_object( &thread->obj );
+    thread->exit_poll = NULL;
+    wake_up( &thread->obj, 0 );
+    release_object( &thread->obj );
+}
+
 /* kill a thread on the spot */
 void kill_thread( struct thread *thread, int violent_death )
 {
@@ -1265,8 +1287,12 @@ void kill_thread( struct thread *thread, int violent_death )
     kill_console_processes( thread, 0 );
     debug_exit_thread( thread );
     abandon_mutexes( thread );
-    wake_up( &thread->obj, 0 );
-    if (violent_death) send_thread_signal( thread, SIGQUIT );
+    if (violent_death)
+    {
+        send_thread_signal( thread, SIGQUIT );
+        check_terminated( thread );
+    }
+    else wake_up( &thread->obj, 0 );
     cleanup_thread( thread );
     remove_process_thread( thread->process, thread );
     release_object( thread );
diff --git a/server/thread.h b/server/thread.h
index 650bc44628d..668cc4ebd79 100644
--- a/server/thread.h
+++ b/server/thread.h
@@ -91,6 +91,7 @@ struct thread
     struct list            kernel_object; /* list of kernel object pointers */
     data_size_t            desc_len;      /* thread description length in bytes */
     WCHAR                 *desc;          /* thread description string */
+    struct timeout_user   *exit_poll;     /* poll if the thread/process has exited already */
 };
 
 extern struct thread *current;
-- 
2.27.0

