[RFC] Thread spawn hook (inheriting thread locals)

[m-ou-se]

Rendered

[m-ou-se]

cc @epage - You said you wanted this for your new test framework. :)

[m-ou-se]

Implementation, including use in libtest: https://github.com/rust-lang/rust/pull/125405

[m-ou-se]

Demonstration:

Code

```rust #![feature(thread_spawn_hook)] use std::cell::Cell; use std::thread; thread_local! { static ID: Cell = panic!("ID not set!"); } fn main() { ID.set(123); thread::add_spawn_hook(|_| { let id = ID.get(); Ok(move || ID.set(id)) }); thread::spawn(|| { println!("1: {}", ID.get()); thread::spawn(|| { println!("1.1: {}", ID.get()); thread::spawn(|| { println!("1.1.1: {}", ID.get()); }).join().unwrap(); thread::spawn(|| { println!("1.1.2: {}", ID.get()); }).join().unwrap(); }).join().unwrap(); thread::spawn(|| { ID.set(456); // <-- change thread local `ID` println!("1.2: {}", ID.get()); thread::spawn(|| { println!("1.2.1: {}", ID.get()); }).join().unwrap(); thread::spawn(|| { println!("1.2.2: {}", ID.get()); }).join().unwrap(); }).join().unwrap(); }).join().unwrap(); thread::spawn(|| { println!("2: {}", ID.get()); }).join().unwrap(); } ```

Output

``` 1: 123 1.1: 123 1.1.1: 123 1.1.2: 123 1.2: 456 1.2.1: 456 1.2.2: 456 2: 123 ```

[the8472]

Could this be made more controllable through thread::Builder? We could make it Clone, modify a global default instance and new() would then clone from the default, giving users the option to modify it further (such as modifying the set of hooks) before spawning additional threads.

[m-ou-se]

Could this be made more controllable through thread::Builder? We could make it Clone, modify a global default instance and new() would then clone from the default, giving users the option to modify it further (such as modifying the set of hooks) before spawning additional threads.

That might make sense for the stack size, but not for the thread name. And those are (at least today) the only two settings that a Builder has.

I don't think it makes sense to allow 'modifying' the set of hooks. If you want to change/remove any hook, you need a way to identify them. Using a number or string as identifier has many issues, and using some kind of ThreadSpawnHookHandle for each of them seems a bit much, without clear use cases..

[the8472]

The default thread name would be empty, as it is today. And people have asked for more features in builders such as affinity or thread priorities (or hooks that can take care of those for specific threads): https://github.com/rust-lang/libs-team/issues/195

I don't think it makes sense to allow 'modifying' the set of hooks. If you want to change/remove any hook, you need a way to identify them.

The current proposal provides add. With the builder we could at least have clear to opt-out for some subset of threads. Since the closures have to be 'static anyway we could just do it based on typeID? The types could be made nameable via existential types.

[m-ou-se]

The current proposal provides add. With the builder we could at least have clear to opt-out for some subset of threads. Since the closures have to be 'static anyway we could just do it based on typeID? The types could be made nameable via existential types.

Why would you want to fully clear the hooks? Without knowing which hooks are registered, you can't know which things you're opting out of, so you basically can't know what clear even means. E.g. would you expect clearing these hooks to break a #[test] with threads?

I strongly believe this should just work like atexit() (global, only registering, no unregistering).

(Even if we were to allow some way to skip the hooks for a specific builder/thread, it doesn't make sense to me to have a global "clear" function to clear all the registered hooks (from the 'global builder' as you propose), because you can't really know which other hooks (of other crates) you might be removing.)

Since the closures have to be 'static anyway we could just do it based on typeID?

TypeId is not enough. Two hooks might have the same type but a different value (and different behaviour), e.g. with different captures. (Also if you register already boxed Box<dyn Fn> as hooks, they will all just have the (same) TypeId of that box type.)

[the8472]

Why would you want to fully clear the hooks? Without knowing which hooks are registered, you can't know which things you're opting out of, so you basically can't know what clear even means.

The RFC already states that threads can be in an unhooked state through pthread spawn. And I assume some weird linkage things (dlopening statically linked plugins?) you could end up with a lot more threads not executing hooks? So unlike atexit it should already be treated as semi-optional behavior.

Additionally I'm somewhat concerned about a tool intended for automatic state inheritance being a global. In the Enterprise Java world such kind of implicit behavior/magic automatisms occasionally get abused to do terribly bloated things which means it's great when that bloat can be limited to a smaller scope or at least be opted out of. For similar increasingly complex inheritable process properties problems linux systems are trying to occasionally shed this kind of state (e.g. sudo vs. systemd's run0, or process spawn servers instead of fork)

In the context of a test framework I can imagine tests running in sets and then clearing global state (or at least enumerating it to print information what didn't get cleaned up) to decouple tests from each other.

E.g. would you expect clearing these hooks to break a #[test] with threads?

"break" in the sense of not doing output capture, as it already is today? Yeah, sure, if the tests need a clean environment for whatever reason that might be a side-effect.

[m-ou-se]

as it already is today?

Today, inheriting the output capturing is done unconditionally when std spawns a thread. There is no way to disable it or bypass it for specific threads (other than just using pthread or something directly). What I'm proposing here is a drop-in replacement for that exact feature, but more flexible so it can be used for other things than just output capturing. It will work in the exact same situations.

I'm not aware of anyone doing anything specifically to avoid inheriting libtest's output capturing (such as using pthread_create directly for that purpose).

Additionally I'm somewhat concerned about a tool intended for automatic state inheritance being a global.

The destructors for e.g. thread_local!{} are also registered globally (depending on the platform). atexit() is global. All these things start with a global building block.

I agree it'd be nice to have some sort of with_thread_local_context(|| { ... }) to create a scope in which all threads spawned will have a certain context, but the building block you need to make that feature is what is proposed in this RFC.

[joshtriplett]

I do think we're likely to want deregistration as well, but I think we'd want to do that via a scoped function rather than naming. I don't think it has to be in the initial version in any case.

EDIT: On second thought, I'm increasingly concerned about not having the ability to remove these hooks.

[ChrisDenton]

Are there use cases other than testing frameworks in mind? Do they have needs that differ from the testing framework case?

[joshtriplett]

I think hooks are going to be quite prone to potential deadlocks. That's not a blocker for doing this, just something that'll need to be documented.

[tmccombs]

Are there use cases other than testing frameworks in mind?

I can think of a few other use cases:

• creating a new span in a tracing framework for the new thread that uses the thread local context of the parent thread as the parent span.
• creating a new logger in TLS with configuration based on the local logger of the parent thread
• inheriting a value for a thread local variable across spawning a new thread could have several use cases
• possibly recording metrics on when threads are started, although that would probably be most useful if there was some way to hook into thread destruction as well.
• Initialize a thread local random number generator seeded by the parent thread's random number generator. I'm not sure if that is advantageous over current approaches to thread local RNGs, but it would be possible with this.

[programmerjake]

• possibly recording metrics on when threads are started, although that would probably be most useful if there was some way to hook into thread destruction as well.

TLS drop implementations could likely be used for that.