Skip to content

Conversation

@tobz
Copy link

@tobz tobz commented Jul 1, 2022

Problem

I'm working on a custom global allocator that tracks allocation and deallocation events for the purpose of tracking memory consumption/usage and attributing it to different components. As such, for fast/uncontended access to update the statistics of a particular component, I use ThreadLocal to hold the necessary data structures, and then scatter/gather them in a background thread.

Since the allocator is intercepting all allocations and deallocations, this means that when TLS destructors are run for ThreadLocal itself, specifically THREAD_HOLDER, it triggers calls into my custom allocator, which result in hitting the normal "get the statistics for the given component" logic, which tries to access the ThreadLocal... and ends up panicking due to the access after destruction.

The custom allocator already tries to avoid reentrantly tracking (de)allocations but since the TLS destructor is the root of the stack, as it were, and there's no way that I know of to check that the key is in fact destroyed, I can't detect what's happening out-of-band, and need the access to THREAD_HOLDER to be able to bubble that up.

Solution

I've added fallible variants of all of the accessor methods: try_get, try_get_or, try_get_or_try, and try_get_or_default. They simply call out to thread::try_get, which is itself a fallible variant of thread::get. Ultimately, we're just bubbling up the already-available AccessError.

Testing

I added two tests for ensuring the fallible try_* variants work, but I was struggling to conceive a unit test that represented my particular example, given the need for a custom allocator to be in place to affect THREAD_HOLDER in the necessary way, and to that end... I wonder if it even necessarily matters: we're really just surfacing existing LocalKey methods.

@Amanieu
Copy link
Owner

Amanieu commented Jul 1, 2022

I don't think this crate is the right place to work around this issue. Have you considered wrapping the ThreadLocal in a wrapper object that sets a thread-local bool flag when it is in used. You could use ManuallyDrop to ensure the flag is set when it is being dropped.

@tobz
Copy link
Author

tobz commented Jul 3, 2022

I guess I'm not sure what you mean when you say "this issue".

The existence of the fallible accessors on LocalKey make a pretty compelling argument that having to handle such cases is not atypical, and the notion of using yet another variable when the TLS destructor information is already available seems silly.

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

Successfully merging this pull request may close these issues.

2 participants