Boris Zbarsky [:bzbarsky] Assignee
	Description • 18 years ago

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....

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 1 • 18 years ago

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

	Graydon Hoare :graydon
	Comment 2 • 18 years ago

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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 4 • 18 years ago

Note that the stack in comment 0 doesn't involve GTK...

	Graydon Hoare :graydon
	Comment 5 • 18 years ago

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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 6 • 18 years ago

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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 7 • 18 years ago

This fixes the asserts too -- at a guess, the free() hook reentered the hashtable code and made it unhappy or something.

	David Baron :dbaron:
	Comment 9 • 18 years ago

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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 11 • 18 years ago

Fixed.

	David Baron :dbaron:
	Comment 12 • 18 years ago

Apparently this caused a Txul change.  How big was it?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 13 • 18 years ago

About 7%.  See bug 372618.