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

Thread Local Allocator Clear-up #9

Closed
ashennell opened this issue Aug 9, 2016 · 2 comments
Closed

Thread Local Allocator Clear-up #9

ashennell opened this issue Aug 9, 2016 · 2 comments
Labels

Comments

@ashennell
Copy link

Hi,

Reading through your code I can't see how your Nifty Counter approach clears up the temporary allocators safely . As the temporary_allocator_dtor_t instances are only static and not thread local themselves, their destructors will only be called at program exit and only on one thread. The temporary_allocator_dtor_t thread local data members will only be accessed on that one thread - so their is no point in them being thread local. Any allocators on other threads, including threads that are killed mid program will never be deallocated.

Perhaps my understanding of your solution is wrong.
Nice library, by the way.

@foonathan
Copy link
Owner

I remember finding the issue and thought I've fixed it.

I think you are right, the dtor_t needs to be thread_local as well, but the GCC __thread keyword that is used for older versions doesn't support thread local objects with non-trivial destructor, so they can't be.

This needs to be fixed, yes.

@foonathan foonathan added the bug label Aug 9, 2016
@foonathan
Copy link
Owner

Due to an issue with thread local under OSX, I'm currently reworking the temporary_allocator completely.

Lifetime control can be more explicit then.

foonathan added a commit that referenced this issue Aug 14, 2016
* restructure `temporary_allocator`, explictly extracted
`temporary_stack`
* improve automatic creation and destruction of `temporary_stack`s
* add comparision to `memory_stack<>::marker`
* add `memory_stack_raii_unwind` utility

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

No branches or pull requests

2 participants