Closed
Bug 1118209
Opened 10 years ago
Closed 3 years ago
TaskTracer doesn't aware that profiler could be initialized multiple times
Categories
(Core :: Gecko Profiler, defect)
Core
Gecko Profiler
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: kanru, Assigned: cyu)
References
Details
Attachments
(1 file)
(deleted),
patch
|
sinker
:
review-
|
Details | Diff | Splinter Review |
The first time is in http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp?rev=32b690a35683#4169 via GeckoProfilerInitRAII
The second time is in http://mxr.mozilla.org/mozilla-central/source/xpcom/build/XPCOMInit.cpp?rev=32b690a35683#494 via profiler_init
Task tracer crashes because a assert on TLS data
Breakpoint 1, mozilla::tasktracer::InitTaskTracer (aFlags=aFlags@entry=0) at ../../../gecko/tools/profiler/GeckoTaskTracer.cpp:208
208 MOZ_ASSERT(!sTraceInfoTLS);
Assignee | ||
Comment 1•10 years ago
|
||
in tools/profiler/platform.cpp:
> int sInitCount = 0; // Each init must have a matched shutdown.
I think the profiler is expected to be init'd multiple times, as long as it has the same number of shutdown calls. I am not sure why it is designed in this way. Maybe it is unavoidable. I guess we need to do the same.
Reporter | ||
Updated•10 years ago
|
Summary: Profiler initialized twice → TaskTracer doesn't aware that profiler could be initialized multiple times
Assignee | ||
Comment 2•10 years ago
|
||
It's not really dependent upon bug 1095433 that much. But the bug has some fundamental changes that I would like to land first so I just it to dependent bug and avoid the patches from stepping on each other's toes.
Assignee: nobody → cyu
Depends on: 1095433
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8545865 -
Flags: review?(tlee)
Comment 4•10 years ago
|
||
Comment on attachment 8545865 [details] [diff] [review]
Allow multiple init/shutdown calls
Review of attachment 8545865 [details] [diff] [review]:
-----------------------------------------------------------------
::: tools/profiler/GeckoTaskTracer.cpp
@@ +228,5 @@
> ShutdownTaskTracer()
> {
> + sInitCount --;
> + if (sInitCount > 0) {
> + return;
This would make the process that call |ReinitTaskTracer()| @ https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild.cpp#629 never shutting down.
Attachment #8545865 -
Flags: review?(tlee) → review-
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Thinker Li [:sinker] from comment #4)
> This would make the process that call |ReinitTaskTracer()| @
> https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild.cpp#629
> never shutting down.
The problem will not happen with bug 1095433 fixed. Calling InitTaskTracer with the flag FORKED_AFTER_NUWA only obseletes current trace info and doesn't bump sInitCount.
Task Tracer was removed in bug 1715257.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•