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

Runtime support for thread-local storage #631

Closed
jonhoo opened this issue Sep 11, 2018 · 2 comments
Closed

Runtime support for thread-local storage #631

jonhoo opened this issue Sep 11, 2018 · 2 comments

Comments

@jonhoo
Copy link
Sponsor Contributor

jonhoo commented Sep 11, 2018

I recently had an issue where my tokio-based application would often crash before exiting with the relatively unhelpful error message:

cannot access a TLS value during or after it is destroyed: AccessError

While my application did use thread-local storage (TLS), it did not use LocalKey.with (which is where this error originates, so I was pretty confused what was causing this. After some gdb debugging, I discovered that the offending with call was located within tokio (fixed in #628). Following some discussion on the tokio gitter channel, @stjepang then pointed out:

@jonhoo: Well, I do store a thing that holds a Runtime inside of a TLS (specifically, in a RefCell). Is that an issue?
@stjepang: That is probably it then :) Well, the Tokio runtime heavily uses TLS and cannot be dropped properly while the TLS is already tearing down.

This came as a surprise to me, and I do not believe it is documented anywhere at the moment. My code did indeed have something that amounts to:

thread_local! {
    static CLIENT: RefCell<Option<tokio::runtime::Runtime>> = RefCell::new(None);
}

Which I set to Some during setup, but never explicitly dropped, instead expecting it to be cleaned up automatically during thread teardown. This is indeed what happened, but apparently tokio doesn't expect to have to do work while thread locals are being destroyed (which is what caused this issue). The fix is pretty straightforward — just explicitly set the thread local to None before the thread exits to trigger a drop — but realizing that this is the issue in the first place requires a lot of debugging.

So, all that to say that I think tokio needs to either make it very clear that tokio and its components cannot be dropped inside of TLS (if so, where do we document this), or we need to fix the tokio components where this is broken (e.g., #593 (comment)) so that this works as expected. While the latter is more appealing as a user, it is also likely a lot more work. I think either is acceptable, since the workaround is also usually quite straightforward.

If we do opt to disallow dropping Runtime (others?) in TLS teardown, then ideally we should also detect it when this happens if possible. I don't know exactly how we'd do that (can you detect that you're in TLS teardown), but if tokio could at that point scream "DON'T DROP ME IN TLS", that would certainly help avoid users spending hours on trying to track down this issue like I had to!

@DoumanAsh
Copy link
Contributor

Ah I concur, I did encounter the same issue and as TLS dtors order is out of control, at the moment at least docs should be updated to state against it.

On other hand there is another option for user.
For example I did worked around this limitation by introducing special struct that cleans up TLS in its dtor:

thread_local!(static TOKIO: Cell<Option<Runtime>> = Cell::new(None));

pub struct Guard {
    //workaround for unstable !Send
    _dummy: PhantomData<*mut u8>
}
impl Drop for Guard {
    fn drop(&mut self) {
        TOKIO.with(|rt| rt.replace(None));
    }
}

You must make sure to not let it be Send, but other than that just by creating at the beggining of main would guarantee you that tokio's Runtime goes out of scope before tokio's internal TLS

@carllerche
Copy link
Member

Closing as a dup of #593.

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

No branches or pull requests

3 participants