Closed
Bug 880165
Opened 11 years ago
Closed 3 years ago
Fix problems to do with sCurrentThreadProfile in platform-linux.cc
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
INACTIVE
People
(Reporter: jseward, Assigned: jseward)
References
Details
Attachments
(1 file)
6.45 KB,
patch
|
Details | Diff | Splinter Review |
Whilst trying to track down races causing crashing in the new
multithreaded profiler, I saw some problems relating to the
global
static ThreadProfile* sCurrentThreadProfile = NULL;
at platform-linux.cc:143
(1) It's NULL/non-NULL status is used to tell SignalSender's loop when
ProfilerSignalHandler has finished. Unfortunately there's a return
path from ProfileSignalHandler that doesn't NULL it out, leaving
SignalSender stuck forever in its sched_yield loop:
if (!Sampler::GetActiveSampler())
return;
I think I have seen this happening.
(2) Since sCurrentThreadProfile is accessed from multiple threads
without locking, we can't just read/write it in the normal way,
since that violates C++11, hence puts us at risk of compiler & hw
reordering. We should wrap it up in the shiny new mfbt/Atomic.h
facilities. Maybe Nathan can advise us on that.
(3) AFAICS, the current use of sCurrentThreadProfile does prevent
races between SignalSender and ProfilerSignalHandler (ignoring
(1) and (2) above). In effect its NULL/non-NULL status is used
as a mutex. However, Helgrind can't see that, and I want to be
able to race check the profiler, so some annotatations will be
necessary.
As a comment .. AFAICS the current scheme unnecessarily serialises
the uses (calls of) of ProfileSignalHandler. At least in principle
it should be able to run in multiple profiled threads simultaneously,
although right now it never will.
Assignee | ||
Comment 1•11 years ago
|
||
Obsolete.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INACTIVE
You need to log in
before you can comment on or make changes to this bug.
Description
•