From 1cb76b7ca484f9429cf21a24e1cbb4f27ec128f0 Mon Sep 17 00:00:00 2001
From: Nadav Har'El <nyh@cloudius-systems.com>
Date: Wed, 17 Apr 2013 23:30:01 +0300
Subject: [PATCH] Fixed two bugs in pthread implementation

1. pthread_join should allow retval=NULL, in which case the return value is
ignored. We therefore need to use void**, not void*&, otherwise passing NULL
causes pthread_join to crash in a strange way.

2. pthread_join forgot to delete the object allocated in pthread_create
---
 libc/pthread.cc   | 12 ++++++++----
 tests/tst-leak.cc | 17 +++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/libc/pthread.cc b/libc/pthread.cc
index dfd295139..995ba6f4d 100644
--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -35,7 +35,7 @@ namespace pthread_private {
         ~pthread() { free_stack(_thread.get_stack_info()); }
         static pthread* from_libc(pthread_t p);
         pthread_t to_libc();
-        int join(void*& retval);
+        int join(void** retval);
         void* _retval;
         // must be initialized last
         sched::thread _thread;
@@ -79,10 +79,12 @@ namespace pthread_private {
         mmu::unmap(si.begin, si.size);
     }
 
-    int pthread::join(void*& retval)
+    int pthread::join(void** retval)
     {
         _thread.join();
-        retval = _retval;
+        if (retval) {
+            *retval = _retval;
+        }
         return 0;
     }
 
@@ -124,7 +126,9 @@ int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
 
 int pthread_join(pthread_t thread, void** retval)
 {
-    return pthread::from_libc(thread)->join(*retval);
+    int ret = pthread::from_libc(thread)->join(retval);
+    delete(pthread::from_libc(thread));
+    return ret;
 }
 
 int pthread_key_create(pthread_key_t* key, void (*dtor)(void*))
diff --git a/tests/tst-leak.cc b/tests/tst-leak.cc
index 12356ef8c..87f858853 100644
--- a/tests/tst-leak.cc
+++ b/tests/tst-leak.cc
@@ -1,11 +1,18 @@
 #include "sched.hh"
 #include "debug.hh"
 #include <sys/mman.h>
+#include <pthread.h>
 
 namespace memory {
 extern bool tracker_enabled;
 }
 
+static void *donothing(void *arg)
+{
+    debug(".");
+    return NULL;
+}
+
 int main(int argc, char **argv)
 {
     bool save_tracker_enabled = memory::tracker_enabled;
@@ -44,6 +51,16 @@ int main(int argc, char **argv)
         t->join();
         delete t;
     }
+
+    debug("testing leaks in pthread_create");
+    for(int i=0; i<10; i++){
+        pthread_t t;
+        assert(pthread_create(&t, NULL, donothing, NULL) == 0);
+        assert(pthread_join(t, NULL) == 0);
+    }
+    debug("\n");
+
+
     debug("leak testing done. Please use 'osv leak show' in gdb to analyze results\n");
 
     memory::tracker_enabled = save_tracker_enabled;
-- 
GitLab