| Summary: | Enhance valgrind to supress unfreed stderr. | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Carlos O'Donell <carlos> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | REPORTED --- | ||
| Severity: | wishlist | CC: | mark, tom |
| Priority: | NOR | ||
| Version: | 3.15 SVN | ||
| Target Milestone: | --- | ||
| Platform: | Fedora RPMs | ||
| OS: | Linux | ||
| See Also: | https://sourceware.org/bugzilla/show_bug.cgi?id=26022 | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
Which process shutdown steps do you expect to run during or after __libc_freeres is called? __libc_freeres as used by valgrind should be the last "user code" running, no other threads should be scheduled. It is called after all user threads are "dead". And after __libc_freeres has ran only the valgrind (tool) code will run (which doesn't use glibc code). So at least from the perspective of valgrind __libc_freeres can release all resources. (In reply to Mark Wielaard from comment #1) > Which process shutdown steps do you expect to run during or after > __libc_freeres is called? Any detached thread may write to the stream until the kernel kills the task. > __libc_freeres as used by valgrind should be the last "user code" running, > no other threads should be scheduled. It is called after all user threads > are "dead". And after __libc_freeres has ran only the valgrind (tool) code > will run (which doesn't use glibc code). How do you wait for detached threads? > So at least from the perspective of valgrind __libc_freeres can release all > resources. So from your perspective at this point, even detached threads, can expect to have the streams shutdown? (In reply to Carlos O'Donell from comment #2) > (In reply to Mark Wielaard from comment #1) > > Which process shutdown steps do you expect to run during or after > > __libc_freeres is called? > > Any detached thread may write to the stream until the kernel kills the task. > > > __libc_freeres as used by valgrind should be the last "user code" running, > > no other threads should be scheduled. It is called after all user threads > > are "dead". And after __libc_freeres has ran only the valgrind (tool) code > > will run (which doesn't use glibc code). > > How do you wait for detached threads? > > > So at least from the perspective of valgrind __libc_freeres can release all > > resources. > > So from your perspective at this point, even detached threads, can expect to > have the streams shutdown? What is a "detached thread" according to you? There is no concept of detached threads in valgrind, all threads are under control of valgrind and valgrind makes sure that only one (kernel) thread is scheduled to run at any one time. See https://valgrind.org/docs/manual/manual-core.html#manual-core.pthreads A detached thread in pthreads is one that is no longer being supervised and which will vanish when it completes rather than waiting to be reaped (joined in pthreads terminology) by another thread. The situation under valgrind is as you say rather different though in that even if pthreads has forgotten about the thread valgrind hasn't. (In reply to Mark Wielaard from comment #3) > What is a "detached thread" according to you? There is no concept of > detached threads in valgrind, all threads are under control of valgrind and > valgrind makes sure that only one (kernel) thread is scheduled to run at any > one time. See > https://valgrind.org/docs/manual/manual-core.html#manual-core.pthreads If you use ptrace under the hood to stop a pthread detached thread, and therefore terminate it, then yes, there will be nothing else using the streams. If that's the case, and we feel comfortable with that as a position then I can flip this back to glibc and discuss *always* shutting down streams via __libc_freeres regardless of their state? (In reply to Carlos O'Donell from comment #5) > (In reply to Mark Wielaard from comment #3) > > What is a "detached thread" according to you? There is no concept of > > detached threads in valgrind, all threads are under control of valgrind and > > valgrind makes sure that only one (kernel) thread is scheduled to run at any > > one time. See > > https://valgrind.org/docs/manual/manual-core.html#manual-core.pthreads > > If you use ptrace under the hood to stop a pthread detached thread, and > therefore terminate it, then yes, there will be nothing else using the > streams. > > If that's the case, and we feel comfortable with that as a position then I > can flip this back to glibc and discuss *always* shutting down streams via > __libc_freeres regardless of their state? Even if we did enhance glibc, there is a need for all existing distributions that have unbuffered stderr which won't be freed by __libc_freeres(). (In reply to Carlos O'Donell from comment #5) > (In reply to Mark Wielaard from comment #3) > > What is a "detached thread" according to you? There is no concept of > > detached threads in valgrind, all threads are under control of valgrind and > > valgrind makes sure that only one (kernel) thread is scheduled to run at any > > one time. See > > https://valgrind.org/docs/manual/manual-core.html#manual-core.pthreads > > If you use ptrace under the hood to stop a pthread detached thread, and > therefore terminate it, then yes, there will be nothing else using the > streams. > > If that's the case, and we feel comfortable with that as a position then I > can flip this back to glibc and discuss *always* shutting down streams via > __libc_freeres regardless of their state? We don't use ptrace under the hood, we control all kernel threads. From the perspective of glibc valgrind provides the kernel interface. Valgrind core doesn't use glibc itself. From the perspective of valgrind glibc is just like any other code the process executes. With some exceptions, we intercept some glibc functions, most specifically __libc_freeres. That function is called after the process exits (from glibc's perspective). We should probably make sure there are no other users of __libc_freeres, or if there are, that they use it in the same way. So it seems a good idea to put this back to glibc. (In reply to Mark Wielaard from comment #7) > (In reply to Carlos O'Donell from comment #5) > > (In reply to Mark Wielaard from comment #3) > > > What is a "detached thread" according to you? There is no concept of > > > detached threads in valgrind, all threads are under control of valgrind and > > > valgrind makes sure that only one (kernel) thread is scheduled to run at any > > > one time. See > > > https://valgrind.org/docs/manual/manual-core.html#manual-core.pthreads > > > > If you use ptrace under the hood to stop a pthread detached thread, and > > therefore terminate it, then yes, there will be nothing else using the > > streams. > > > > If that's the case, and we feel comfortable with that as a position then I > > can flip this back to glibc and discuss *always* shutting down streams via > > __libc_freeres regardless of their state? > > We don't use ptrace under the hood, we control all kernel threads. From the > perspective of glibc valgrind provides the kernel interface. Valgrind core > doesn't use glibc itself. From the perspective of valgrind glibc is just > like any other code the process executes. With some exceptions, we intercept > some glibc functions, most specifically __libc_freeres. That function is > called after the process exits (from glibc's perspective). We should > probably make sure there are no other users of __libc_freeres, or if there > are, that they use it in the same way. So it seems a good idea to put this > back to glibc. There are other users of __libc_freeres(), particularly mtrace, which frees everything in a destructors, but that problably makes such code incompatible with valgrind and that's OK. Let me note, that even if we fix it in glibc, that is to say that we decree that all streams, buffered or unbuffered, are freed, there is going to be old glibc's and applications that use stderr (unbuffered) and allocate a buffer (possibly a 20 year old glibc bug) that is not freed. I leave it up to you to decide if you want to supress an unfreed stderr that will always show up when using stderr. I'll see about fixing it in glibc. |
SUMMARY In glibc the __libc_freeres() function which handles freeing all resources for allocation trackers, will not free unbuffered streams. We don't free unbuffered streams because users may expect to continue using them throughout all of the process shutdown steps. As such, when you use stderr in your program it creates a leak that can be seen by users. STEPS TO REPRODUCE 1. Create leak.c #include <stdio.h> #include <wchar.h> int main ( void ) { fwprintf ( stderr , L"valgrind error\n" ) ; return 0 ; } 2. Compile. 3. Run with valgrind --leak-check=full --show-leak-kinds=all OBSERVED RESULT valgrind --leak-check=full --show-leak-kinds=all ./leak ==258071== Memcheck, a memory error detector ==258071== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==258071== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==258071== Command: ./leak ==258071== valgrind error ==258071== ==258071== HEAP SUMMARY: ==258071== in use at exit: 5,120 bytes in 2 blocks ==258071== total heap usage: 2 allocs, 0 frees, 5,120 bytes allocated ==258071== ==258071== 1,024 bytes in 1 blocks are still reachable in loss record 1 of 2 ==258071== at 0x483A809: malloc (vg_replace_malloc.c:309) ==258071== by 0x4907FC3: _IO_file_doallocate (filedoalloc.c:101) ==258071== by 0x490940C: _IO_wfile_doallocate (wfiledoalloc.c:70) ==258071== by 0x4916D1F: _IO_doallocbuf (genops.c:347) ==258071== by 0x4916D1F: _IO_doallocbuf (genops.c:342) ==258071== by 0x490FBBF: _IO_wfile_overflow (wfileops.c:429) ==258071== by 0x490D22E: __woverflow (wgenops.c:217) ==258071== by 0x490D22E: _IO_wdefault_xsputn (wgenops.c:317) ==258071== by 0x490D22E: _IO_wdefault_xsputn (wgenops.c:284) ==258071== by 0x490FDF0: _IO_wfile_xsputn (wfileops.c:1011) ==258071== by 0x490FDF0: _IO_wfile_xsputn (wfileops.c:957) ==258071== by 0x4906BC1: buffered_vfprintf (vfprintf-internal.c:2388) ==258071== by 0x4903E4B: __vfwprintf_internal (vfprintf-internal.c:1346) ==258071== by 0x490C439: fwprintf (fwprintf.c:33) ==258071== by 0x401152: main (leak.c:9) ==258071== ==258071== 4,096 bytes in 1 blocks are still reachable in loss record 2 of 2 ==258071== at 0x483A809: malloc (vg_replace_malloc.c:309) ==258071== by 0x49093DC: _IO_wfile_doallocate (wfiledoalloc.c:79) ==258071== by 0x4916D1F: _IO_doallocbuf (genops.c:347) ==258071== by 0x4916D1F: _IO_doallocbuf (genops.c:342) ==258071== by 0x490FBBF: _IO_wfile_overflow (wfileops.c:429) ==258071== by 0x490D22E: __woverflow (wgenops.c:217) ==258071== by 0x490D22E: _IO_wdefault_xsputn (wgenops.c:317) ==258071== by 0x490D22E: _IO_wdefault_xsputn (wgenops.c:284) ==258071== by 0x490FDF0: _IO_wfile_xsputn (wfileops.c:1011) ==258071== by 0x490FDF0: _IO_wfile_xsputn (wfileops.c:957) ==258071== by 0x4906BC1: buffered_vfprintf (vfprintf-internal.c:2388) ==258071== by 0x4903E4B: __vfwprintf_internal (vfprintf-internal.c:1346) ==258071== by 0x490C439: fwprintf (fwprintf.c:33) ==258071== by 0x401152: main (leak.c:9) ==258071== ==258071== LEAK SUMMARY: ==258071== definitely lost: 0 bytes in 0 blocks ==258071== indirectly lost: 0 bytes in 0 blocks ==258071== possibly lost: 0 bytes in 0 blocks ==258071== still reachable: 5,120 bytes in 2 blocks ==258071== suppressed: 0 bytes in 0 blocks ==258071== ==258071== For lists of detected and suppressed errors, rerun with: -s ==258071== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) EXPECTED RESULT - Given that stderr might be used a lot by developers we could enhance valgrind to try supress unfreed unbuffered stderr at process exit. - For a high QoI we would supress all unfreed unbuffered streams since the expectation, at least in glibc, is that none of these are freed with __libc_freeres() because the could continue to be used during process shutdown. SOFTWARE/OS VERSIONS Fedora 32: valgrind-3.15.0-20.fc32.x86_64 ADDITIONAL INFORMATION N/A