Skip to content
New issue

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

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

Already on GitHub? # to your account

fixed unnecessary set_specific in _mi_heap_set_default_direct #977

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

rubensturm1
Copy link

an unnecessary call to mi_prim_thread_associate_default_heap is made even if MI_TLS_PTHREAD is not defined. This causes unnecessary problems for applications that use separate linker namespaces as pthread_keys are bugged for those applications.

@res2k
Copy link
Contributor

res2k commented Dec 25, 2024

mi_prim_thread_associate_default_heap() appears to defined for more cases besides pthreads, but would, after this change, only be called for MI_TLS_PTHREAD, so this seems incorrect.

@rubensturm1
Copy link
Author

I see what you mean, it seems to not use pthreads for windows applications. however for other applications it just calls pthread_setspecific with the _mi_heap_default_key, but this value is only retrieved by mi_prim_get_default_heap if MI_TLS_PTHREAD is defined. Otherwise, this value is retrieved using other methods from other locations. So I guess it should be in a (#elif defined(MI_TLS_PTHREAD)||defined(_WIN32)) block?

@daanx
Copy link
Collaborator

daanx commented Dec 30, 2024

Hi -- I think if we want to avoid the call to pthread_specific, we should do that in src/prim/unix/prim.c in the _mi_prim_thread_associate_default_heap method (by testing if MI_TLS_PTHREAD is defined). However, we still need the pthread local even if MI_TLS_PTHREAD is not defined in order to get a callback on thread termination. Also, it is unclear to me why we need to avoid calling pthread_set_specific ? I am not familiar with linker namespaces and the bug with pthreads -- can you provide a link or some background? Thanks!

@rubensturm1
Copy link
Author

Thank you for your response. For applications that use separate linker namespaces, calling pthread_set_specific can override data stored by the pthread_set_specific of another namespace as they don't share which keys are taken but do share the memory they use https://sourceware.org/bugzilla/show_bug.cgi?id=26955. Calling pthread_set_specific unnecessarily here can break these applications even when they could work fine.

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

Successfully merging this pull request may close these issues.

4 participants