Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Description • 15 years ago

According to bsmedberg, it should be

 (1) initialized as part of NS_LogInit()
 (2) choose its log files through the environment rather than command-line flags
 (3) have its symbols be libxul-internal rather than exported

Also it should be closed down with NS_LogTerm().

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 1 • 15 years ago

I just hit bug 525573 for an e10s unit test (IPDL), for the first time.  This is pretty scary; it means all the non-firefox-bin e10s programs, including child processes, are susceptible to this threadsafety problem in tracemalloc builds and we're playing with fire.  Definitely need to hook this into LogInit()/LogTerm()

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 2 • 15 years ago

Partial implementation of comment 0 ... frankly, the most important part.  The other steps can be completed later.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 3 • 15 years ago

Comment on attachment 413463 [details] [diff] [review]
"Start" trace-malloc in LogInit(), it hasn't already been started

Not sure whose realm this falls into.

	David Baron :dbaron:
	Comment 5 • 15 years ago

After looking at the existing code, I'm curious what threadsafety problems you're running into that result from not having started trace-malloc.

If you don't call anything in trace-malloc, then:
 * we should never hook malloc at all on Mac and Windows (StartupHooker should never be called)
 * the Linux malloc hooks should all return almost immediately on discovering tracing_enabled is false

Is that not actually the case?  Why is it necessary to start trace-malloc?

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 6 • 15 years ago

The new deadlock detector uses trace-malloc to get backtraces in DEBUG builds.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 7 • 15 years ago

I should add that the old deadlock detector also used trace-malloc to get backtraces.  It was just by luck that a multi-threaded XPCOM unit test never went into an infinite loop like I've seen.

	David Baron :dbaron:
	Comment 8 • 15 years ago

I'd actually written this before seeing your last two comments:

Or is the threadsafety problem you're hitting related to callers of NS_TraceMallocGetStackTrace() ?  Perhaps a better fix for those issues would be to make the tm_thread* argument to backtrace() be optional, since there's no need to suppress tracing or log if trace-malloc is disabled (I'm not particularly happy with what I did in bug 442192, and that would probably be a better solution).

	David Baron :dbaron:
	Comment 9 • 15 years ago

Oh, I guess calltree actually needs the lock.  So I guess this is the way to go.

	David Baron :dbaron:
	Comment 10 • 15 years ago

Comment on attachment 413463 [details] [diff] [review]
"Start" trace-malloc in LogInit(), it hasn't already been started

r=dbaron

I guess the idea of exposing NS_TraceMallocStackTrace() to callers outside trace-malloc was a clever idea that wasn't actually as simple as it looked.  Oh well; I think it's still useful.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 11 • 15 years ago

Pushed http://hg.mozilla.org/mozilla-central/rev/7c5e126880cf