Fault() called on startup, causing cycle collector to not work

RESOLVED FIXED

Status

()

defect
--
critical
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

BUILD: Current trunk Firefox build

STEPS TO REPRODUCE:
1)  firefox -g
2)  break 'Fault(char const*, void const*)' 
3)  Say "yes" to set the future breakpoint.
4)  run

EXPECTED RESULTS: Breakpoint is not hit

ACTUAL RESULTS:
(gdb) where
#0  0xb7db040c in Fault (msg=0xb7df0350 "freed while purple", ptr=0x0)
    at ../../../mozilla/xpcom/base/nsCycleCollector.cpp:573
#1  0xb7db1a25 in nsCycleCollector::Freed (this=0xb7e00ce0, n=0x0)
    at ../../../mozilla/xpcom/base/nsCycleCollector.cpp:1593
#2  0xb7db0fd4 in my_free_hook (ptr=0x0, caller=0xb7c63b57)
    at ../../../mozilla/xpcom/base/nsCycleCollector.cpp:1243
#3  0xb7b6bde5 in free () from /lib/tls/libc.so.6
#4  0xb7c63b57 in PR_Free (ptr=0x0)
    at ../../../../../mozilla/nsprpub/pr/src/malloc/prmem.c:490
#5  0xb7dac7da in NS_Free_P (ptr=0x0) at ../../../mozilla/xpcom/base/nsMemoryImpl.cpp:303
#6  0xb7fdd9e2 in SetAllocatedString (str=@0xbffff1f8, newvalue=0x80486f4 "Mozilla")
    at ../../../mozilla/toolkit/xre/nsAppData.cpp:47
#7  0xb7fddbe4 in ScopedAppData (this=0xbffff1f0, aAppData=0x8049780)
    at ../../../mozilla/toolkit/xre/nsAppData.cpp:72
#8  0xb7fcec02 in XRE_main (argc=1, argv=0xbffff314, aAppData=0x8049780)
    at ../../../mozilla/toolkit/xre/nsAppRunner.cpp:2003
#9  0x0804860a in main (argc=1, argv=0xbffff314)
    at ../../../mozilla/browser/app/nsBrowserApp.cpp:61

This sets sCollector.mParams.mDoNothing, so we never actually do any collecting.

I think we should run some tinderbox with XPCOM_CC_FAULT_IS_FATAL set to catch this when it happens....
Flags: blocking1.9?
This also happens in Seamonkey, with the stack:

(gdb) wher
#0  0xb7f8a40c in Fault (msg=0xb7fca350 "freed while purple", ptr=0x0)
    at ../../../mozilla/xpcom/base/nsCycleCollector.cpp:573
#1  0xb7f8ba25 in nsCycleCollector::Freed (this=0xb7fdace0, n=0x0)
    at ../../../mozilla/xpcom/base/nsCycleCollector.cpp:1593
#2  0xb7f8afd4 in my_free_hook (ptr=0x0, caller=0xb7c204dd)
    at ../../../mozilla/xpcom/base/nsCycleCollector.cpp:1243
#3  0xb7c63de5 in free () from /lib/tls/libc.so.6
#4  0xb7c204dd in setlocale () from /lib/tls/libc.so.6
#5  0x0037632d in gtk_parse_args () from /usr/lib/libgtk-x11-2.0.so.0
#6  0x003763ec in gtk_init_check () from /usr/lib/libgtk-x11-2.0.so.0
#7  0x00376432 in gtk_init () from /usr/lib/libgtk-x11-2.0.so.0
#8  0x08051bb1 in main (argc=1, argv=0xbffff304)
    at ../../../mozilla/xpfe/bootstrap/nsAppRunner.cpp:1623
This is the second reported case of GTK interacting poorly with the cycle collector's free hooks. I'd recommend disabling them entirely. They have no relevance to the actual cycle collection algorithm; they're just sanity checks. I'll prepare a patch to turn them off.
Status: NEW → ASSIGNED
Posted patch This seems to do the job (obsolete) — Splinter Review
Attachment #253126 - Flags: superreview?(jonas)
Attachment #253126 - Flags: review?
(Assignee)

Updated

12 years ago
Attachment #253126 - Flags: review? → review?(graydon)
Note that the stack in comment 0 doesn't involve GTK...
Flags: blocking1.9? → blocking1.9+
I think it'd be more appropriate to make the free hook ignore null pointers and make the purple buffer fault on null pointers. They should never make it that far.
Posted patch Patch like that (obsolete) — Splinter Review
I didn't put in the Fault() because I don't really want to spend the cycles checking the pointer twice.  But maybe we should...

Also note the XXX comment.  I think we shouldn't land it -- we should either remove the setting of __malloc_initialize_hook or change the comment to reflect how things work; I just wasn't sure which we wanted.
Attachment #253274 - Flags: review?(graydon)
This fixes the asserts too -- at a guess, the free() hook reentered the hashtable code and made it unhappy or something.
Attachment #253126 - Attachment is obsolete: true
Attachment #253274 - Attachment is obsolete: true
Attachment #253276 - Flags: superreview?(jonas)
Attachment #253276 - Flags: review?(graydon)
Attachment #253274 - Flags: review?(graydon)
Attachment #253126 - Flags: superreview?(jonas)
Attachment #253126 - Flags: review?(graydon)
Attachment #253276 - Flags: review?(graydon) → review+
Assignee: nobody → graydon
Status: ASSIGNED → NEW
(Assignee)

Updated

12 years ago
Duplicate of this bug: 368528
Comment on attachment 253276 [details] [diff] [review]
Per discussion with graydon

sr=dbaron, although I think nsPurpleBuffer needs to have rules on zeros.  I'll file a separate bug on that.
Attachment #253276 - Flags: superreview?(jonas) → superreview+
Fixed.
Assignee: graydon → bzbarsky
(Assignee)

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Depends on: 372618
Apparently this caused a Txul change.  How big was it?
About 7%.  See bug 372618.
You need to log in before you can comment on or make changes to this bug.