[RFC PATCH v2] loader: allow overriding pthread_create correctly on linux

Maarten Lankhorst m.b.lankhorst at gmail.com
Fri Jul 5 05:31:37 CDT 2013


Op 28-06-13 12:53, Maarten Lankhorst schreef:
> This should be enough to fix bug #30557 in the most thorough possible way.
> Only tested on ubuntu precise 32-bits atm, but I believe it should work on 64-bits too.
>
> TODO: pthread_create_2_0 is not handled correctly right now, so it will fail on ancient glibc's < 2.2.
> But because NPTL requires a slightly newer glibc than that I do not believe it will be a real issue.
>
> Comments welcome :-)

So v1 had some issues because pthread_join can not be called multiple times.

Some hooking of pthread_join and pthread_detach later.. combined with robust mutex abuse fixes
this particular race. Currently I don't handle pthread_tryjoin_np and pthread_timedjoin_np,
but I do not think they're called as often.

When the thread dies, it takes the thread_lock. This makes sure no threads are caught in an intermediary
dying stage, but will be thoroughly dead. EOWNERDEAD signals a thread death, in which case pthread_join
is performed to clean up the resources if the thread was detached.

When a thread dies, it will die with the thread_lock held, making sure only 1 thread is dying at the same
time and no intermediary state is possible. All native threads are put on the active list, unless they're
detached. pthread_join removes them from the list and causes them to clean up. :-)

8< -----
diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h
index 146ce50..75c98da 100644
--- a/dlls/ntdll/ntdll_misc.h
+++ b/dlls/ntdll/ntdll_misc.h
@@ -28,6 +28,7 @@
 #include "winnt.h"
 #include "winternl.h"
 #include "wine/server.h"
+#include "wine/list.h"
 
 #define MAX_NT_PATH_LENGTH 277
 
@@ -236,6 +237,8 @@ struct ntdll_thread_data
     WINE_VM86_TEB_INFO vm86;          /* 1fc vm86 private data */
     void              *exit_frame;    /* 204 exit frame pointer */
 #endif
+    struct list entry;
+    BOOL detached;
 };
 
 static inline struct ntdll_thread_data *ntdll_get_thread_data(void)
diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c
index 01a8026..8944189 100644
--- a/dlls/ntdll/thread.c
+++ b/dlls/ntdll/thread.c
@@ -33,6 +33,7 @@
 #ifdef HAVE_SYS_SYSCALL_H
 #include <sys/syscall.h>
 #endif
+#include <errno.h>
 
 #define NONAMELESSUNION
 #include "ntstatus.h"
@@ -58,6 +59,7 @@ struct startup_info
     TEB                            *teb;
     PRTL_THREAD_START_ROUTINE       entry_point;
     void                           *entry_arg;
+    BOOL                            native_thread;
 };
 
 static PEB *peb;
@@ -186,6 +188,62 @@ done:
     return status;
 }
 
+#ifdef __linux__
+extern typeof(pthread_create) *__glob_pthread_create, *call_pthread_create;
+extern typeof(pthread_join) *__glob_pthread_join, *call_pthread_join;
+extern typeof(pthread_detach) *__glob_pthread_detach, *call_pthread_detach;
+
+static typeof(pthread_create) __hook_pthread_create;
+static typeof(pthread_join) __hook_pthread_join;
+static typeof(pthread_detach) __hook_pthread_detach;
+
+static pthread_mutex_t thread_lock;
+
+static void thread_wrap_init(void)
+{
+    pthread_mutexattr_t attr;
+    pthread_mutexattr_init(&attr);
+    pthread_mutexattr_setrobust(&attr, PTHREAD_MUTEX_ROBUST);
+    pthread_mutex_init(&thread_lock, &attr);
+    pthread_mutexattr_destroy(&attr);
+
+    call_pthread_create = __hook_pthread_create;
+    call_pthread_join = __hook_pthread_join;
+    call_pthread_detach = __hook_pthread_detach;
+}
+
+static TEB *dead_teb;
+static struct list active_list = LIST_INIT(active_list);
+
+static void take_thread_lock(void)
+{
+    int ret = pthread_mutex_lock(&thread_lock);
+    if (ret == EOWNERDEAD && dead_teb) {
+        struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)dead_teb->SpareBytes1;
+
+        __glob_pthread_join(thread_data->pthread_id, NULL);
+        signal_free_thread(dead_teb);
+        dead_teb = NULL;
+    }
+    if (ret == EOWNERDEAD)
+        pthread_mutex_consistent(&thread_lock);
+}
+
+static void reap_thread(TEB *teb)
+{
+    struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)teb->SpareBytes1;
+    take_thread_lock();
+    if (thread_data->detached)
+        dead_teb = teb;
+}
+
+#else
+#define __glob_pthread_create pthread_create
+#define __glob_pthread_join pthread_join
+#define __glob_pthread_detach pthread_detach
+#define thread_wrap_init()
+#endif
+
 /***********************************************************************
  *           thread_init
  *
@@ -204,6 +262,7 @@ HANDLE thread_init(void)
     struct ntdll_thread_data *thread_data;
     static struct debug_info debug_info;  /* debug info for initial thread */
 
+    thread_wrap_init();
     virtual_init();
 
     /* reserve space for shared user data */
@@ -327,14 +386,12 @@ void terminate_thread( int status )
     pthread_exit( UIntToPtr(status) );
 }
 
-
-/***********************************************************************
- *           exit_thread
- */
-void exit_thread( int status )
+static void exit_thread_common( int status )
 {
+#ifndef __linux__
     static void *prev_teb;
     TEB *teb;
+#endif
 
     if (status)  /* send the exit code to the server (0 is already the default) */
     {
@@ -362,24 +419,158 @@ void exit_thread( int status )
 
     pthread_sigmask( SIG_BLOCK, &server_block_set, NULL );
 
+#ifndef __linux__
     if ((teb = interlocked_xchg_ptr( &prev_teb, NtCurrentTeb() )))
     {
         struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)teb->SpareBytes1;
 
         if (thread_data->pthread_id)
         {
-            pthread_join( thread_data->pthread_id, NULL );
+            __glob_pthread_join( thread_data->pthread_id, NULL );
             signal_free_thread( teb );
         }
     }
+#else
+    reap_thread(NtCurrentTeb());
+#endif
 
     close( ntdll_get_thread_data()->wait_fd[0] );
     close( ntdll_get_thread_data()->wait_fd[1] );
     close( ntdll_get_thread_data()->reply_fd );
     close( ntdll_get_thread_data()->request_fd );
+}
+
+void exit_thread( int status )
+{
+    exit_thread_common(status);
     pthread_exit( UIntToPtr(status) );
 }
 
+#ifdef __linux__
+
+struct unix_arg {
+    void *(*start)(void *);
+    void *arg;
+};
+
+/* dummy used for comparison */
+static DWORD native_unix_start;
+
+static void call_native_cleanup(void *arg)
+{
+    RtlFreeThreadActivationContextStack();
+    exit_thread_common(0);
+}
+
+static int
+__hook_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+		      void *(*start_routine) (void *), void *parm)
+{
+    NTSTATUS ret;
+    size_t stack = 8 * 1024 * 1024;
+    struct unix_arg arg;
+    arg.start = start_routine;
+    arg.arg = parm;
+
+    TRACE("Overriding thread creation!\n");
+    if (attr) {
+        static int once;
+        if (!once++)
+            FIXME("most thread attributes ignored!\n");
+        else
+            WARN("most thread attributes ignored!\n");
+
+        pthread_attr_getstacksize(attr, &stack);
+    }
+
+    ret = RtlCreateUserThread( NtCurrentProcess(), NULL, FALSE, NULL, stack, 0, (void*)&native_unix_start, &arg, NULL, (void*)thread );
+    if (ret != STATUS_SUCCESS)
+        FIXME("ret: %08x\n", ret);
+    switch (ret) {
+    case STATUS_SUCCESS:
+        return 0;
+    case STATUS_NO_MEMORY:
+        return ENOMEM;
+    case STATUS_TOO_MANY_OPENED_FILES:
+        return EMFILE;
+    default:
+        ERR("Unhandled ntstatus %08x\n", ret);
+        return ENOMEM;
+    }
+}
+
+static int __hook_pthread_detach(pthread_t thread)
+{
+    struct ntdll_thread_data *thread_data;
+    TEB *teb = NULL;
+
+    if (pthread_equal(thread, pthread_self())) {
+        ntdll_get_thread_data()->detached = 1;
+        return 0;
+    }
+
+    take_thread_lock();
+    LIST_FOR_EACH_ENTRY(thread_data, &active_list, typeof(*thread_data), entry) {
+        if (pthread_equal(thread_data->pthread_id, thread)) {
+            teb = CONTAINING_RECORD(thread_data, typeof(*teb), SpareBytes1);
+
+            list_remove(&thread_data->entry);
+            if (!pthread_tryjoin_np(thread, NULL))
+                signal_free_thread(teb);
+            else
+                thread_data->detached = 1;
+            break;
+        }
+    }
+    pthread_mutex_unlock(&thread_lock);
+    return teb ? 0 : ESRCH;
+}
+
+static int __hook_pthread_join(pthread_t thread, void **retval)
+{
+    struct ntdll_thread_data *thread_data, *t2;
+
+    if (pthread_equal(thread, pthread_self()))
+        return EDEADLK;
+
+    take_thread_lock();
+    LIST_FOR_EACH_ENTRY(thread_data, &active_list, typeof(*thread_data), entry) {
+        TEB *teb = CONTAINING_RECORD(thread_data, typeof(*teb), SpareBytes1);
+
+        if (pthread_equal(thread, thread_data->pthread_id)) {
+            int ret;
+
+            ret = pthread_tryjoin_np(thread, retval);
+            if (!ret)
+                list_remove(&thread_data->entry);
+            pthread_mutex_unlock(&thread_lock);
+
+            if (!ret) {
+                signal_free_thread(teb);
+                return 0;
+            }
+
+            if ((ret = __glob_pthread_join(thread, retval)))
+                return ret;
+
+            take_thread_lock();
+            /* Check if someone else freed the thread yet */
+            LIST_FOR_EACH_ENTRY(t2, &active_list, typeof(*thread_data), entry)
+                if (t2 == thread_data) {
+                    list_remove(&thread_data->entry);
+                    signal_free_thread(teb);
+                    break;
+                }
+            pthread_mutex_unlock(&thread_lock);
+            return 0;
+        }
+    }
+
+    pthread_mutex_unlock(&thread_lock);
+    return ESRCH;
+}
+
+#endif
 
 /***********************************************************************
  *           start_thread
@@ -412,9 +603,26 @@ static void start_thread( struct startup_info *info )
     if (TRACE_ON(relay))
         DPRINTF( "%04x:Starting thread proc %p (arg=%p)\n", GetCurrentThreadId(), func, arg );
 
-    call_thread_entry_point( (LPTHREAD_START_ROUTINE)func, arg );
-}
+#ifdef __linux__
+    if (info->native_thread) {
+        void *(*start)(void*) = (void*)func;
 
+        thread_data->detached = 0;
+
+        take_thread_lock();
+        list_add_tail(&active_list, &thread_data->entry);
+        pthread_mutex_unlock(&thread_lock);
+
+        FIXME("Started native thread %08x\n", GetCurrentThreadId());
+        pthread_cleanup_push(call_native_cleanup, NULL);
+        pthread_exit(start(arg));
+        pthread_cleanup_pop(1);
+        return;
+    }
+    thread_data->detached = 1;
+#endif
+        call_thread_entry_point( (LPTHREAD_START_ROUTINE)func, arg );
+}
 
 /***********************************************************************
  *              RtlCreateUserThread   (NTDLL.@)
@@ -497,8 +705,18 @@ NTSTATUS WINAPI RtlCreateUserThread( HANDLE process, const SECURITY_DESCRIPTOR *
 
     info = (struct startup_info *)(teb + 1);
     info->teb         = teb;
-    info->entry_point = start;
-    info->entry_arg   = param;
+#ifdef __linux__
+    info->native_thread = (void*)start == (void*)&native_unix_start;
+    if (info->native_thread) {
+        struct unix_arg *arg = param;
+        info->entry_point = (void*)arg->start;
+        info->entry_arg = arg->arg;
+    } else
+#endif
+    {
+        info->entry_point = start;
+        info->entry_arg   = param;
+    }
 
     thread_data = (struct ntdll_thread_data *)teb->SpareBytes1;
     thread_data->request_fd  = request_pipe[1];
@@ -513,7 +731,7 @@ NTSTATUS WINAPI RtlCreateUserThread( HANDLE process, const SECURITY_DESCRIPTOR *
                            (char *)teb->Tib.StackBase - (char *)teb->DeallocationStack );
     pthread_attr_setscope( &attr, PTHREAD_SCOPE_SYSTEM ); /* force creating a kernel thread */
     interlocked_xchg_add( &nb_threads, 1 );
-    if (pthread_create( &pthread_id, &attr, (void * (*)(void *))start_thread, info ))
+    if (__glob_pthread_create( &pthread_id, &attr, (void * (*)(void *))start_thread, info ))
     {
         interlocked_xchg_add( &nb_threads, -1 );
         pthread_attr_destroy( &attr );
@@ -523,6 +741,11 @@ NTSTATUS WINAPI RtlCreateUserThread( HANDLE process, const SECURITY_DESCRIPTOR *
     pthread_attr_destroy( &attr );
     pthread_sigmask( SIG_SETMASK, &sigset, NULL );
 
+#ifdef __linux__
+    if ((void*)start == (void*)&native_unix_start && id)
+        *(pthread_t*)id = pthread_id;
+    else
+#endif
     if (id) id->UniqueThread = ULongToHandle(tid);
     if (handle_ptr) *handle_ptr = handle;
     else NtClose( handle );
diff --git a/dlls/winegstreamer/glibthread.c b/dlls/winegstreamer/glibthread.c
index 25eceb8..7417941 100644
--- a/dlls/winegstreamer/glibthread.c
+++ b/dlls/winegstreamer/glibthread.c
@@ -43,6 +43,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 
+#if 0
 #include "windef.h"
 #include "winbase.h"
 #include "winnls.h"
@@ -388,3 +389,15 @@ void g_thread_impl_init (void)
   g_thread_self_tls = TlsAlloc ();
   g_thread_init(&g_thread_functions_for_glib_use_default);
 }
+
+#else
+
+void g_thread_impl_init (void)
+{
+  static gboolean beenhere = FALSE;
+
+  if (!beenhere++)
+    g_thread_init(NULL);
+}
+
+#endif
diff --git a/libs/wine/loader.c b/libs/wine/loader.c
index 094e5e1..5d67c4c 100644
--- a/libs/wine/loader.c
+++ b/libs/wine/loader.c
@@ -69,6 +69,13 @@ char **__wine_main_argv = NULL;
 WCHAR **__wine_main_wargv = NULL;
 char **__wine_main_environ = NULL;
 
+#ifdef __linux__
+#include <pthread.h>
+typeof(pthread_create) *call_pthread_create, *__glob_pthread_create;
+typeof(pthread_join) *call_pthread_join, *__glob_pthread_join;
+typeof(pthread_detach) *call_pthread_detach, *__glob_pthread_detach;
+#endif
+
 struct dll_path_context
 {
     unsigned int index; /* current index in the dll path list */
diff --git a/libs/wine/wine.map b/libs/wine/wine.map
index 2159fac..fb3b951 100644
--- a/libs/wine/wine.map
+++ b/libs/wine/wine.map
@@ -117,6 +117,12 @@ WINE_1.0
     wine_utf8_mbstowcs;
     wine_utf8_wcstombs;
     wine_wctype_table;
+    __glob_pthread_create;
+    call_pthread_create;
+    __glob_pthread_join;
+    call_pthread_join;
+    __glob_pthread_detach;
+    call_pthread_detach;
 
   local: *;
 };
diff --git a/loader/main.c b/loader/main.c
index ac67290..e2410c4 100644
--- a/loader/main.c
+++ b/loader/main.c
@@ -202,6 +202,42 @@ static int pre_exec(void)
 
 #endif
 
+#ifdef __linux__
+
+extern typeof(pthread_create) *call_pthread_create, *__glob_pthread_create;
+extern typeof(pthread_detach) *call_pthread_detach, *__glob_pthread_detach;
+extern typeof(pthread_join) *call_pthread_join, *__glob_pthread_join;
+
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+		   void *(*start_routine) (void *), void *arg)
+{
+    return call_pthread_create(thread, attr, start_routine, arg);
+}
+
+int pthread_detach(pthread_t thread)
+{
+    return call_pthread_detach(thread);
+}
+
+int pthread_join(pthread_t thread, void **retval)
+{
+    return call_pthread_join(thread, retval);
+}
+
+static void init_thread_hook(void) {
+    call_pthread_create = __glob_pthread_create = dlvsym(RTLD_NEXT, "pthread_create", "GLIBC_2.2.5");
+    if (!__glob_pthread_create)
+        call_pthread_create = __glob_pthread_create = dlvsym(RTLD_NEXT, "pthread_create", "GLIBC_2.1");
+
+    call_pthread_detach = __glob_pthread_detach = dlsym(RTLD_NEXT, "pthread_detach");
+    call_pthread_join = __glob_pthread_join = dlsym(RTLD_NEXT, "pthread_join");
+}
+
+#else
+
+#define init_thread_hook()
+
+#endif
 
 /**********************************************************************
  *           main
@@ -211,6 +247,8 @@ int main( int argc, char *argv[] )
     char error[1024];
     int i;
 
+    init_thread_hook();
+
     if (!getenv( "WINELOADERNOEXEC" ))  /* first time around */
     {
         static char noexec[] = "WINELOADERNOEXEC=1";




More information about the wine-patches mailing list