Closed Bug 529134 Opened 11 years ago Closed 11 years ago
Change the way tracemalloc is initialized
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().
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()
Partial implementation of comment 0 ... frankly, the most important part. The other steps can be completed later.
Assignee: nobody → jones.chris.g
Attachment #413463 - Flags: review?(dbaron)
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.
Attachment #413463 - Flags: review?(benjamin)
11 years ago
Duplicate of this bug: 525573
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?
The new deadlock detector uses trace-malloc to get backtraces in DEBUG builds.
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.
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).
Oh, I guess calltree actually needs the lock. So I guess this is the way to go.
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.
Attachment #413463 - Flags: review?(dbaron) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.