Hi,

I was debugging on Hurd a misbehaviour of muntrace, which would just 
spin taking 100% CPU: using a simple test like:
--vvvvv--
#include <mcheck.h>
int main()
{ mtrace(); muntrace(); return 0; }
--^^^^^--
when run as `MALLOC_TRACE=out test`, you get a backtrace like the 
attached trace.log (that one has been produced with debian's eglibc 
2.13, but it does the same with current glibc).

What happens is the following:
a) muntrace gets called
b) in muntrace, "= End\n" is written to the file, and fclose on it is
   called
c) in fclose, free is called to free the FILE*, triggering the free hook
   (which is still set)
d) in tr_freehook, lock_and_info is called which locks the lock, and
   then tr_where is called
e) in tr_where, fprintf in called, which at some point calls
   _IO_file_doallocate
and at this point there is the behaviour difference between Linux and 
Hurd: in Linux EXEC_PAGESIZE is provided (by sys/param.h, coming from 
linux/param.h), thus in libio/libioP.h _G_HAVE_MMAP is kept, so 
ALLOC_BUF and FREE_BUF (used in _IO_file_doallocate) use mmap/munmap. On 
the Hurd, however, EXEC_PAGESIZE is provided nowhere (and this causes 
build issues also in two files under elf/), so the two _BUF macros use 
malloc/free... which during the muntrace execution in turn calls the 
malloc hook (which is still set), and then lock_and_info tries to lock 
the lock -> deadlock.

Ignoring the fact that in libio mmap is not used on Hurd (it will need a 
different fix), it seems to me this whole hook triggering during 
muntrace seems more harmful than useful (on Linux it is attempted to 
output the log for the free of mallstream, which always fails since that 
stream is closed at that point), so my attached proposal is to first 
unset mallstream and the hooks, and only after that close the file.

Thanks,
-- 
Pino Toscano
#0  0x0106281c in swtch_pri () at /build/buildd-eglibc_2.13-36-hurd-i386-IvO_gk/eglibc-2.13/build-tree/hurd-i386-libc/mach/swtch_pri.S:2
#1  0x010640a4 in __spin_lock_solid (lock=0x11ded74) at spin-solid.c:27
#2  0x0106429d in __mutex_lock_solid (lock=0x11ded74) at mutex-solid.c:31
#3  0x010eb328 in __mutex_lock (__lock=<optimized out>) at ../mach/lock-intern.h:89
#4  tr_mallochook (size=8192, caller=0x10d690f) at mtrace.c:170
#5  0x010e92ff in __libc_malloc (bytes=8192) at malloc.c:3622
#6  0x010d690f in _IO_file_doallocate (fp=0x804a0e8) at filedoalloc.c:120
#7  0x010e1ec8 in _IO_doallocbuf (fp=0x804a0e8) at genops.c:423
#8  0x010e11db in _IO_new_file_overflow (f=0x804a0e8, ch=-1) at fileops.c:850
#9  0x010e04f3 in _IO_new_file_xsputn (f=0x804a0e8, data=0x11c4665, n=2) at fileops.c:1358
#10 0x010b642e in _IO_vfprintf_internal (s=0x804a0e8, format=0x11c4665 "@ %s%s%s[%p] ", ap=0x1024cf0 "Ч\002\001\344I\034\001") at vfprintf.c:1336
#11 0x010c024b in __fprintf (stream=0x804a0e8, format=0x11c4665 "@ %s%s%s[%p] ") at fprintf.c:33
#12 0x010eb0e0 in tr_where (caller=0x10d6a5c) at mtrace.c:127
#13 0x010eb3a6 in tr_freehook (ptr=0x804a0e8, caller=0x10d6a5c) at mtrace.c:146
#14 0x010e914c in __libc_free (mem=0x1) at malloc.c:3699
#15 0x010d6a5c in _IO_new_fclose (fp=0x804a0e8) at iofclose.c:88
#16 0x010eadcc in muntrace () at mtrace.c:364
#17 0x0804852c in main () at mt.c:3
muntrace: reset file and hooks before finalizing the stream

fclose will call free, invoking its hook, then fprintf which would indirectly
try to allocate a buffer, and this can cause malloc to be used (thus its hook
to be invoked) if libio uses malloc instead of mmap; given any malloc/free hook
locks the internal lock, this leads to a deadlock.

To prevent this hook roundtrip at muntrace, first unset MALLSTREAM and the
hooks, and only after that close the trace file.

2012-11-17  Pino Toscano  <toscano.p...@tiscali.it>

	* malloc/mtrace.c (muntrace): Reset MALLSTREAM and the hooks before
	finalizing MALLSTREAM.
--- a/malloc/mtrace.c
+++ b/malloc/mtrace.c
@@ -361,14 +361,18 @@ mtrace ()
 void
 muntrace ()
 {
+  FILE *f;
+
   if (mallstream == NULL)
     return;
 
-  fprintf (mallstream, "= End\n");
-  fclose (mallstream);
+  f = mallstream;
   mallstream = NULL;
   __free_hook = tr_old_free_hook;
   __malloc_hook = tr_old_malloc_hook;
   __realloc_hook = tr_old_realloc_hook;
   __memalign_hook = tr_old_memalign_hook;
+
+  fprintf (f, "= End\n");
+  fclose (f);
 }

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to