Merge pull request #103 from jacob-hughes/fix_threads

Switch to using Boehm's pthread drop-in library

Cargo.lock

@@ -311,6 +311,7 @@ dependencies = [
  "cmake",
  "compiler_builtins",
  "core",
+ "libc",
 ]
 
 [[package]]

library/alloc/src/gc.rs

@@ -112,16 +112,6 @@ pub fn init() {
     unsafe { boehm::GC_init() }
 }
 
-/// Returns true if thread was successfully registered.
-pub unsafe fn register_thread(stack_base: *mut u8) -> bool {
-    unsafe { boehm::GC_register_my_thread(stack_base) == 0 }
-}
-
-/// Returns true if thread was successfully unregistered.
-pub unsafe fn unregister_thread() -> bool {
-    unsafe { boehm::GC_unregister_my_thread() == 0 }
-}
-
 pub fn suppress_warnings() {
     unsafe { boehm::GC_set_warn_proc(&boehm::GC_ignore_warn_proc as *const _ as *mut u8) };
 }

library/boehm/Cargo.toml

@@ -1,3 +1,5 @@
+cargo-features = ["public-dependency"]
+
 [package]
 name = "boehm"
 version = "0.1.0"
@@ -8,6 +10,7 @@ license = "Apache-2.0 OR MIT"
 [dependencies]
 core = { path = "../core" }
 compiler_builtins = { version = "0.1.10", features = ['rustc-dep-of-std'] }
+libc = { version = "0.2.148", default-features = false, features = ['rustc-dep-of-std'], public = true }
 
 [build-dependencies]
 cmake = "0.1"

library/boehm/src/lib.rs

@@ -1,5 +1,7 @@
 #![no_std]
 
+use libc;
+
 #[repr(C)]
 #[derive(Default)]
 pub struct ProfileStats {
@@ -60,9 +62,18 @@ extern "C" {
 
     pub fn GC_thread_is_registered() -> u32;
 
-    pub fn GC_register_my_thread(stack_base: *mut u8) -> i32;
+    pub fn GC_pthread_create(
+        native: *mut libc::pthread_t,
+        attr: *const libc::pthread_attr_t,
+        f: extern "C" fn(_: *mut libc::c_void) -> *mut libc::c_void,
+        value: *mut libc::c_void,
+    ) -> libc::c_int;
+
+    pub fn GC_pthread_join(native: libc::pthread_t, value: *mut *mut libc::c_void) -> libc::c_int;
+
+    pub fn GC_pthread_exit(value: *mut libc::c_void) -> !;
 
-    pub fn GC_unregister_my_thread() -> i32;
+    pub fn GC_pthread_detach(thread: libc::pthread_t) -> libc::c_int;
 
     pub fn GC_init();
 

library/std/src/sys/unix/thread.rs

@@ -85,7 +85,7 @@ impl Thread {
             };
         }
 
-        let ret = libc::pthread_create(&mut native, &attr, thread_start, p as *mut _);
+        let ret = alloc::boehm::GC_pthread_create(&mut native, &attr, thread_start, p as *mut _);
         // Note: if the thread creation fails and this assert fails, then p will
         // be leaked. However, an alternative design could cause double-free
         // which is clearly worse.
@@ -105,6 +105,8 @@ impl Thread {
                 // Next, set up our stack overflow handler which may get triggered if we run
                 // out of stack.
                 let _handler = stack_overflow::Handler::new();
+
+                debug_assert!(alloc::gc::thread_registered());
                 // Finally, let's run some code.
                 Box::from_raw(main as *mut Box<dyn FnOnce()>)();
             }
@@ -265,7 +267,7 @@ impl Thread {
 
     pub fn join(self) {
         unsafe {
-            let ret = libc::pthread_join(self.id, ptr::null_mut());
+            let ret = alloc::boehm::GC_pthread_join(self.id, ptr::null_mut());
             mem::forget(self);
             assert!(ret == 0, "failed to join thread: {}", io::Error::from_raw_os_error(ret));
         }
@@ -284,7 +286,7 @@ impl Thread {
 
 impl Drop for Thread {
     fn drop(&mut self) {
-        let ret = unsafe { libc::pthread_detach(self.id) };
+        let ret = unsafe { alloc::boehm::GC_pthread_detach(self.id) };
         debug_assert_eq!(ret, 0);
     }
 }

library/std/src/thread/mod.rs

@@ -517,15 +517,6 @@ impl Builder {
                 imp::Thread::set_name(name);
             }
 
-            // SAFETY: Register the thread with libgc so that its stack can be scanned
-            // for garbage collection.
-            let stack_start = unsafe { imp::guard::get_stack_start().unwrap() };
-            if stack_start != crate::ptr::null_mut() {
-                unsafe {
-                    alloc::gc::register_thread(&stack_start as *const _ as *mut u8);
-                }
-            }
-
             crate::io::set_output_capture(output_capture);
 
             // SAFETY: we constructed `f` initialized.
@@ -538,11 +529,6 @@ impl Builder {
                 crate::sys_common::backtrace::__rust_begin_short_backtrace(f)
             }));
 
-            // SAFETY: The thread has no more work to do, so can be unregisterd.
-            unsafe {
-                alloc::gc::unregister_thread();
-            }
-
             // SAFETY: `their_packet` as been built just above and moved by the
             // closure (it is an Arc<...>) and `my_packet` will be stored in the
             // same `JoinInner` as this closure meaning the mutation will be