92281 - trace-malloc standalone re-enters malloc monitor

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Chris Waterson]

In an email from Brian Moore <[e-mail]>, Brian says:

----
Date: Tue, 24 Jul 2001 21:43:59 -0700 (PDT)
From: Brian Moore <[e-mail]>
To: [e-mail]
Subject: trace-malloc

hello brendan,

i am in the middle of adapting your trace-malloc code to stand alone from
mozilla. doing this. i have found a bug.

the bug is that tmmon is deleted from NS_TraceMallocShutdown() and in
order to do this, we partially destroy it and then attempt to enter it.  
tmmon, in this case, is 0x805d698, which is the monitor we are attempting
to both destroy and enter.

#0  PR_Lock (lock=0x805d69c) at ptsynch.c:184
#1  0x40028b27 in PR_EnterMonitor (mon=0x805d698) at ptsynch.c:504
#2  0x400206de in free (ptr=0x805d698) at nsTraceMalloc.c:1383
#3  0x40024625 in PR_Free (ptr=0x805d698) at prmem.c:82
#4  0x40028a7b in PR_DestroyMonitor (mon=0x805d698) at ptsynch.c:478
#5  0x40020b2f in NS_TraceMallocShutdown () at nsTraceMalloc.c:1562
#6  0x4035b25a in exit (status=0) at exit.c:57
#7  0x403529d1 in __libc_start_main () at ../sysdeps/generic/libc-start.c:92

by the time we enter PR_Lock, the lock field of the monitor has been
totally garbefied:

#1  0x40028b27 in PR_EnterMonitor (mon=0x805d698) at ptsynch.c:504
(gdb) print mon->lock
$13 = {mutex = {__m_reserved = -1347440721, __m_count = -1347440721, 
    __m_owner = 0xafafafaf, __m_kind = -1347440721, __m_lock = {
      __status = -1347440721, __spinlock = -1347440721}}, notified = {
    length = -1347440721, cv = {{cv = 0xafafafaf, times = -1347440721}, {
        cv = 0xafafafaf, times = -1347440721}, {cv = 0xafafafaf, 
        times = -1347440721}, {cv = 0xafafafaf, times = -1347440721}, {
        cv = 0xafafafaf, times = -1347440721}, {cv = 0xafafafaf, 
        times = -1347440721}}, link = 0xafafafaf}, locked = -1347440721, 
  owner = 2947526575}

i've fixed this in my tree with the following patch to revision 1.21 of
/mozilla/xpcom/base/nsTraceMalloc.c:

1378c1382
< if (tmmon)
---
> if (tmmon && (tmmon != ptr))

1397c1401
< if (tmmon)
---
> if (tmmon && (tmmon != ptr))


i think you may have a better idea, but i'm presenting this in case you
don't. i appreciate your input. thanks much in advance,

b

ps: if you're interested in the stand-alone build, let me know. it wasn't
much work, but it may be valuable to you if other people ask for it.

----

I'm not exactly sure _why_ this would work in mozilla -- do we just leak the
monitor in that case?

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Yeah, I've had the following fix for this in my tree for a while (it's on bug
84831):

Index: xpcom/base/nsTraceMalloc.c
===================================================================
RCS file: /cvsroot/mozilla/xpcom/base/nsTraceMalloc.c,v
retrieving revision 1.21
diff -u -d -r1.21 nsTraceMalloc.c
--- nsTraceMalloc.c	2001/01/25 22:52:59	1.21
+++ nsTraceMalloc.c	2001/06/28 04:51:58
@@ -1555,8 +1555,9 @@
         }
     }
     if (tmmon) {
-        PR_DestroyMonitor(tmmon);
+        PRMonitor *mon = tmmon;
         tmmon = NULL;
+        PR_DestroyMonitor(mon);
     }
 #ifdef XP_WIN32
     ShutdownHooker();
@@ -1682,7 +1701,7 @@

Comment 2

[Brendan Eich [:brendan]]

The FASTLOAD_20010703_BRANCH, which I hope to land in the 0.9.3 frozen trunk
pref-disabled (for wide QA thanks to Asa and his merry mob), has dbaron's exact
patch, plus a newline on the last line of nsTraceMalloc.c (mjudge seems to have
nuked the one I left there).

/be

Comment 3

[Brendan Eich [:brendan]]

Fix went in with FASTLOAD branch landing.

/be