qchbai / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Perftools should be careful not to call exit from within exit #376

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Force a RAW_CHECK, CHECK, PCHECK, etc to be invoked from a code path that 
executes after main has finished.

What is the expected behaviour? What do you see instead?
I would expect the associated log/error message to be printed and the program 
to exit gracefully. Instead the program hangs indefinitely.

What version of the product are you using? On what operating system?
google-perftools-1.8.3

Please provide any additional information below.
Calls to exit should be replaced with calls to _exit (if possible) for all 
types of fatal conditions. For example, from src/base/logging.h:

 92 // This takes a message to print.  The name is historical.
 93 #define RAW_CHECK(condition, message)                                          \
 94   do {                                                                         \
 95     if (!(condition)) {                                                        \
 96       WRITE_TO_STDERR("Check failed: " #condition ": " message "\n",           \
 97                       sizeof("Check failed: " #condition ": " message "\n")-1);\
 98       exit(1);                                                                 \
 99     }                                                                          \
100   } while (0)

And here you can see the troubling backtrace calling into exit twice which 
produces undefined behaviour:

Check failed: !do_main_heap_check: should have done it

Breakpoint 4, __cxa_finalize (dso=0x0) at /usr/src/lib/libc/stdlib/atexit.c:164
164             _MUTEX_LOCK(&atexit_mutex);
(gdb) bt
#0  __cxa_finalize (dso=0x0) at /usr/src/lib/libc/stdlib/atexit.c:164
#1  0x0000000800bfc462 in exit (status=1) at /usr/src/lib/libc/stdlib/exit.c:67
#2  0x000000080086fd4c in HeapLeakChecker_AfterDestructors () at 
src/heap-checker.cc:2240
#3  0x0000000800c740a6 in __cxa_finalize (dso=0x80098b420) at 
/usr/src/lib/libc/stdlib/atexit.c:181
#4  0x00000008008666a3 in __do_global_dtors_aux () from 
/usr/local/lib/libtcmalloc.so.2
#5  0x0000000800886625 in _fini () from /usr/local/lib/libtcmalloc.so.2
#6  0x000000080062f1d0 in ?? () from /libexec/ld-elf.so.1
#7  0x0000000800507d21 in ?? () from /libexec/ld-elf.so.1
#8  0x0000000800507ede in ?? () from /libexec/ld-elf.so.1
#9  0x0000000800c740b6 in __cxa_finalize (dso=0x0) at 
/usr/src/lib/libc/stdlib/atexit.c:183
#10 0x0000000800bfc462 in exit (status=0) at /usr/src/lib/libc/stdlib/exit.c:67
#11 0x0000000000400745 in _start ()

Original issue reported on code.google.com by chapp...@gmail.com on 1 Nov 2011 at 5:09

GoogleCodeExporter commented 9 years ago
Ah, good find.  Do you have a proposed fix?  I wonder why this hasn't bitten us 
before.  Maybe we don't typically fatal-log in global destructors?

Original comment by csilv...@gmail.com on 2 Nov 2011 at 1:30

GoogleCodeExporter commented 9 years ago
Looks like the fix is to not call exit from any terminal check. So pretty much 
convert all calls to exit in logging.h to _exit.

Original comment by chapp...@gmail.com on 2 Nov 2011 at 3:26

GoogleCodeExporter commented 9 years ago
Makes sense.  I can't think of any downside to this, though that doesn't mean 
there aren't any.  Would you like to draw up a patch?  (btw, I can't remember 
if you've signed the CLA already or not.)

Original comment by csilv...@gmail.com on 2 Nov 2011 at 3:39

GoogleCodeExporter commented 9 years ago

Original comment by csilv...@gmail.com on 4 Nov 2011 at 9:35

GoogleCodeExporter commented 9 years ago
This should be fixed in perftools 1.9, just released. I changed the code to 
call abort() everywhere, rather than abort() in some places and exit() in 
others. I think the exit() calls were just errors.

Original comment by csilv...@gmail.com on 23 Dec 2011 at 12:50