mimalloc icon indicating copy to clipboard operation
mimalloc copied to clipboard

fixed unnecessary set_specific in _mi_heap_set_default_direct

Open rubensturm1 opened this issue 1 year ago • 4 comments

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.

rubensturm1 avatar Dec 24 '24 14:12 rubensturm1

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.

res2k avatar Dec 25 '24 02:12 res2k

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?

rubensturm1 avatar Dec 25 '24 03:12 rubensturm1

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!

daanx avatar Dec 30 '24 18:12 daanx

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.

rubensturm1 avatar Dec 30 '24 19:12 rubensturm1