From c815a3f3d4e8292c0fcd29663085d4e14ac45be5 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Mon, 4 Dec 2023 17:43:00 +0000 Subject: [PATCH] Switch to using Boehm's pthread drop-in library This prevents us from having to register threads explicitly, and can remove a potential source of errors where we supply an incorrect stack bottom. --- Cargo.lock | 1 + library/alloc/src/gc.rs | 10 ---------- library/boehm/Cargo.toml | 3 +++ library/boehm/src/lib.rs | 15 +++++++++++++-- library/std/src/sys/unix/thread.rs | 8 +++++--- library/std/src/thread/mod.rs | 14 -------------- 6 files changed, 22 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 343730d5df150..10df224ead628 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -311,6 +311,7 @@ dependencies = [ "cmake", "compiler_builtins", "core", + "libc", ] [[package]] diff --git a/library/alloc/src/gc.rs b/library/alloc/src/gc.rs index 3efe75164a0ef..5a5c2910cb60f 100644 --- a/library/alloc/src/gc.rs +++ b/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) }; } diff --git a/library/boehm/Cargo.toml b/library/boehm/Cargo.toml index cac2e6dcde441..522b7febf3cfe 100644 --- a/library/boehm/Cargo.toml +++ b/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" diff --git a/library/boehm/src/lib.rs b/library/boehm/src/lib.rs index 50f46b354505c..f1b8c08dd30c7 100644 --- a/library/boehm/src/lib.rs +++ b/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(); diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 806ee1cdf9a61..ff132c65ec5eb 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/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)(); } @@ -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); } } diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 67db88b8c4599..0de9d6c33f96a 100644 --- a/library/std/src/thread/mod.rs +++ b/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