[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #9 from Iain Sandoe --- (In reply to Florian Weimer from comment #8) > Which version of the manual page are you looking at? > > https://man7.org/linux/man-pages/man3/pthread_cleanup_push.3.html seems > pretty clear about the scope-based nature (search for discussion of > break/return/goto). yeah, got it; one needs to read the union of the sections (the page I was reading was slightly different but the same basic info). I suppose if we were able to create a wrapper around the thread routine and the cleanup was NOP for cases without nested fns. Otherwise, it looks a bit tricky for platforms without thread_atexit support. Have to think some more.
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #8 from Florian Weimer --- Which version of the manual page are you looking at? https://man7.org/linux/man-pages/man3/pthread_cleanup_push.3.html seems pretty clear about the scope-based nature (search for discussion of break/return/goto).
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #7 from Iain Sandoe --- (In reply to Florian Weimer from comment #6) > Sorry, pthread_cleanup_push is purely scope-based, like the existing > handler. It cannot be used to push a handler to some unscoped cleanup > function list that persists even after the current function returns. It's > also implemented as a macro, so it's not possible to emit it from builtin > expansion. Ah, then we have a documentation issue, because man pthread_cleanup_push(3) describes running the registered functions on thread cancellation or on thread_exit() [but not, unfortunately if the thread exits by returning - so still not ideal].
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #6 from Florian Weimer --- Sorry, pthread_cleanup_push is purely scope-based, like the existing handler. It cannot be used to push a handler to some unscoped cleanup function list that persists even after the current function returns. It's also implemented as a macro, so it's not possible to emit it from builtin expansion.
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #5 from Iain Sandoe --- (In reply to Florian Weimer from comment #4) > (In reply to Iain Sandoe from comment #3) > > for platforms using pthreads as the underlying resource, then perhaps we can > > do this without thread_atexit (which I do not see in many places) by using > > pthread_cleanup_push () > > The current implementation already uses the same underlying mechanism as > pthread_cleanup_push if building with -fexceptions. It does not solve the > leak because the outermost handler deliberately does not perform a full > deallocation of the thread state. hmm.. I'm slightly confused here. We certainly make the __gcc_nested_func_ptr_deleted () call a cleanup attached to scope exits and certainly the last page of trampolines is not deallocated (as you note for the sake of avoiding churn in m-mapping). However, in the current code the only pthread-specific stuff I see (in, say config/i386/heap-trampoline.c) is specific to changing protections on the mapped pages. What I was thinking of is attaching a thread exit cleanup using pthread_cleanup_push() for platforms with pthreads but without Libc support for _thread_atexit - I guess I'm missing something :)
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #4 from Florian Weimer --- (In reply to Iain Sandoe from comment #3) > for platforms using pthreads as the underlying resource, then perhaps we can > do this without thread_atexit (which I do not see in many places) by using > pthread_cleanup_push () The current implementation already uses the same underlying mechanism as pthread_cleanup_push if building with -fexceptions. It does not solve the leak because the outermost handler deliberately does not perform a full deallocation of the thread state.
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #3 from Iain Sandoe --- for platforms using pthreads as the underlying resource, then perhaps we can do this without thread_atexit (which I do not see in many places) by using pthread_cleanup_push () not sure if gthr.h abstracts that in any way (I do not have a patch at the moment).
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 --- Comment #2 from Florian Weimer --- (In reply to Iain Sandoe from comment #1) > (In reply to Florian Weimer from comment #0) > > > The fix is to register a TLS destructor to > > deallocate that page, too. On glibc, that also fixes another memory leak for > > -fno-exceptions compilations (the default for C) if pthread_exit is called > > with an active trampoline. > > Does this mean you have a proposed patch already? (before I start > investigation) No, this was a reference to __cxa_thread_atexit, which is unfortunately in libstdc++ (but is a thin shim around __cxa_thread_atexit_impl for current glibc). The pthread_key_create fallback probably needs to be duplicated into libgcc_s.
[Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401 Iain Sandoe changed: What|Removed |Added CC||iains at gcc dot gnu.org --- Comment #1 from Iain Sandoe --- (In reply to Florian Weimer from comment #0) > The fix is to register a TLS destructor to > deallocate that page, too. On glibc, that also fixes another memory leak for > -fno-exceptions compilations (the default for C) if pthread_exit is called > with an active trampoline. Does this mean you have a proposed patch already? (before I start investigation)
