Closed
Bug 529134
Opened 15 years ago
Closed 15 years ago
Change the way tracemalloc is initialized
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
People
(Reporter: cjones, Assigned: cjones)
References
Details
Attachments
(1 file)
(deleted),
patch
|
dbaron
:
review+
benjamin
:
review+
|
Details | Diff | Splinter Review |
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().
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()
Assignee | ||
Comment 2•15 years ago
|
||
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)
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.
Attachment #413463 -
Flags: review?(benjamin)
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?
Assignee | ||
Comment 6•15 years ago
|
||
The new deadlock detector uses trace-malloc to get backtraces in DEBUG builds.
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.
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).
Comment 9•15 years ago
|
||
Oh, I guess calltree actually needs the lock. So I guess this is the way to go.
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.
Attachment #413463 -
Flags: review?(dbaron) → review+
Updated•15 years ago
|
Attachment #413463 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 11•15 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/7c5e126880cf
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•