Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call thread-local variable destructors on thread exit #19

Closed
AzureMarker opened this issue Aug 11, 2022 · 6 comments · Fixed by #28 or #29
Closed

Call thread-local variable destructors on thread exit #19

AzureMarker opened this issue Aug 11, 2022 · 6 comments · Fixed by #28 or #29
Labels
enhancement New feature or request

Comments

@AzureMarker
Copy link
Member

Currently we don't call thread local variable destructors when the thread exits. We should probably fix this.

@Meziu
Copy link
Member

Meziu commented Mar 16, 2023

Those are now called by the default implementation in std

@Meziu Meziu closed this as completed Mar 16, 2023
@AzureMarker
Copy link
Member Author

Oh? Do you have a link I can look at?

@ian-h-chamberlain
Copy link
Member

Hmm, I thought this would be the case too, since we have "has-thread-local": true in the latest nightly, but I tried to test it using a custom Drop impl in the thread-locals example (with upstream std), and it seems like the destructor isn't running.

I would have expected the dtor to be registered here: https://github.com/rust-lang/rust/blob/master/library/std/src/sys/common/thread_local/fast_local.rs#L52

We might want to reopen this and investigate more; I'm not sure what is the best way to prove that std is doing the right thing, but at minimum we should be able to see a destructor running from a spawned thread (not necessarily the main thread, based on this note from std docs).

@Meziu
Copy link
Member

Meziu commented Apr 15, 2023

I've done some research, and I've noticed 2 things:

  1. It seems we are actually using the faster thread_local! implementation, so that's nice. This also means that the pthread-3ds impls aren't used (except in register_dtor_fallback, which explicitly uses the OS implementation).
  2. The destructors, registered in register_dtor_fallback, don't run.

It's hard to understand why, even with gdb at hand. It's pretty clear, however, that there isn't much to change in pthread-3ds, since the TLS keys don't originate here (unless it's an issue with the list held by register_dtor_fallback?).
Otherwise, it's possible the fallback implementation just doesn't fit Horizon (kernel issues or whatnot), but that's an hypothesis hard to verify.

@AzureMarker
Copy link
Member Author

I'll need to refresh myself on the current state of our thread local implementation, but I thought we had to explicitly call the thread destructors when the thread gets destroyed by us? Around here maybe?

@Meziu
Copy link
Member

Meziu commented Apr 21, 2023

You are correct. I had started looking at the problem with the misconception that std handled the destructors by itself in the thread code, but it's (rightfully so) up to the OS implementation to run them. I've started implementing the changes locally, but it seems I'm getting in all sorts of new errors. I'll publish my work if I manage to get the system working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants