Register new threads with the collector #31
Conversation
New thread creation is bottlenecked through `Thread::new`, so this is the ideal place to register its stack as a rootset with the collector.
Multithreaded GC needs to do some collector setup before any mutator work is done.
|
Can we add a multi-threaded test to the test suite to verify that this works (and stays working)? |
|
So that's what |
|
Shouldn't we have a test which does some malloc'ing and freeing to check it doesn't bork? I assume that, before this PR, such code would actively fail? |
|
Yeah that would be ideal, but it's not possible to do this in rustc though because the actual |
OK, that's fine. I'll merge this in advance, because that'll hopefully make your life a bit easier. bors r+ |
|
Thanks! |
|
Build failed: |
55: Fix bug where bool in `thread_registered` was inverted r=ltratt a=jacob-hughes This unblocks softdevteam/alloy#31. Co-authored-by: Jake Hughes <[email protected]>
|
This is a bug in libgc. I remember fixing this locally but left the fix in my working tree for some reason 🤦. Once that merges, this will be unblocked. |
|
bors r+ |
|
Build failed: |
|
Ah! That's an interesting one. This is failing on rustc's gdb tests, they work by: compiling something; prodding around in gdb; and then diffing gdb's output. I permanently disabled this locally because arch-linux packages a subtly different gdb version which formats things differently in stdout. That's why this would have been missed by bors. The actual issue here is pretty straightforward: in the |
|
Ouch -- OK! |
54: Fix the concurrency semantics for `Gc<T>`. r=ltratt a=jacob-hughes This makes two major changes to the API: 1. It removes the requirement that `T: Sync` for `Gc<T>`. 2. It makes `Gc<T> : Send + Sync` if `T: Send + Sync`, fixing the ergonomic problems raised in #49. `Sync`'s purpose is to ensure that two threads can access the data in `T` in a thread-safe way. In other words, it implies that `T` has synchronisation guarantees. Originally, this was added as a constraint on `T` because any finalizer for `T` would run on a separate thread. However, it's now safe to remove this as a constraint because softdevteam/alloy#30 guarantees that a finalizer won't run early. This means that even without synchronized access, a race can't happen, because it's impossible for the finalizer to access `T`'s data while it's still in use by the mutator. However, even though `Gc<T>` can now implement `Send` -- [thanks to multi-threaded collection support](softdevteam/alloy#31) -- `Gc<T>` still requires that `T: Send`, because `T` could be a type with shared ownership which aliases. This is necessary because off-thread finalization could mutate shared memory without synchronisation. An example using `Rc` makes this clear: ```rust struct Inner(Rc<usize>); fn foo() { let rc = Rc::new(123); { let gc = Gc::new(Inner::new(Rc::clone(rc))); } // Assume `gc` is dead here, so it will be finalized in parallel on a separate thread. // This means `Rc::drop` gets called which has the potential to race with // any `Rc` increment / decrement on the main thread. force_gc(); // Might race with gc's finalizer bar(Rc::clone(rc)); } ``` Since finalizing any non-`Send` value can cause UB, we still disallow the construction of `Gc<T: !Send>` completely. This is certainly the most conservative approach. There are others: - Not invoking finalizers for non-`Send` values. This is valid, since finalizers are not guaranteed to run. However, it's not exactly practical, it would mean that any `Gc<Rc<...>>` type structure would _always_ leak. - Finalize `!Send` values on their mutator thread. This is really dangerous in the general case, because if any locks are held on the shared data by the mutator, this will deadlock (it's basically a variant of the async-signal-safe problem). However, if `T` is `!Send`, deadlocking is unlikely [although not impossible!], and could be an acceptable compromise. It's out of the scope of this PR, but it's something I've been playing around with. Co-authored-by: Jake Hughes <[email protected]>
Stepping through a debugger is more difficult now Boehm has its own SIGSEGV handler. These tests are deprecated anyway, and the rust team are working on removing them. More info: rust-lang#47163
|
This is ready for merging again. |
|
bors r+ |
|
Build succeeded: |
31: Fix build script to build boehm with `use_boehm` flag r=ltratt a=jacob-hughes I had forgotten switch this over in 8e8501, and because my dependencies were cached locally (and on the build server!), I didn't notice. Co-authored-by: Jacob Hughes <[email protected]>
No description provided.