Bug 1572337: Don't call TimeStamp::Now() within SuspendAndSample r=froydnj

Avoids deadlocks on Windows due to Now() taking a lock; if done while we've
paused a thread that holds the lock we will deadlock.

Differential Revision: https://phabricator.services.mozilla.com/D52392

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Randell Jesup 2019-11-08 21:18:06 +00:00
parent 2bdf27dbd9
commit 3d6c5b76dd
10 changed files with 45 additions and 28 deletions

View File

@ -288,7 +288,7 @@ void Sampler::Disable(PSLockRef aLock) {
template <typename Func>
void Sampler::SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs) {
const TimeStamp& aNow, const Func& aProcessRegs) {
// Only one sampler thread can be sampling at once. So we expect to have
// complete control over |sSigHandlerCoordinator|.
MOZ_ASSERT(!sSigHandlerCoordinator);
@ -343,7 +343,7 @@ void Sampler::SuspendAndSampleAndResumeThread(
// Extract the current register values.
Registers regs;
PopulateRegsFromContext(regs, &sSigHandlerCoordinator->mUContext);
aProcessRegs(regs);
aProcessRegs(regs, aNow);
//----------------------------------------------------------------//
// Resume the target thread.

View File

@ -81,7 +81,7 @@ void Sampler::Disable(PSLockRef aLock) {}
template <typename Func>
void Sampler::SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs) {
const TimeStamp& aNow, const Func& aProcessRegs) {
thread_act_t samplee_thread =
aRegisteredThread.GetPlatformData()->ProfiledThread();
@ -124,7 +124,7 @@ void Sampler::SuspendAndSampleAndResumeThread(
regs.mFP = reinterpret_cast<Address>(state.REGISTER_FIELD(bp));
regs.mLR = 0;
aProcessRegs(regs);
aProcessRegs(regs, aNow);
}
#undef REGISTER_FIELD

View File

@ -128,7 +128,7 @@ void Sampler::Disable(PSLockRef aLock) {}
template <typename Func>
void Sampler::SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs) {
const TimeStamp& aNow, const Func& aProcessRegs) {
HANDLE profiled_thread =
aRegisteredThread.GetPlatformData()->ProfiledThread();
if (profiled_thread == nullptr) {
@ -174,7 +174,7 @@ void Sampler::SuspendAndSampleAndResumeThread(
Registers regs;
PopulateRegsFromContext(regs, &context);
aProcessRegs(regs);
aProcessRegs(regs, aNow);
//----------------------------------------------------------------//
// Resume the target thread.

View File

@ -1934,7 +1934,7 @@ class Sampler {
template <typename Func>
void SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs);
const TimeStamp& aNow, const Func& aProcessRegs);
private:
# if defined(GP_OS_linux) || defined(GP_OS_android)
@ -2134,7 +2134,8 @@ void SamplerThread::Run() {
buffer.AddEntry(ProfileBufferEntry::Time(delta.ToMilliseconds()));
mSampler.SuspendAndSampleAndResumeThread(
lock, *registeredThread, [&](const Registers& aRegs) {
lock, *registeredThread, now,
[&](const Registers& aRegs, const TimeStamp& aNow) {
DoPeriodicSample(lock, *registeredThread, *profiledThreadData,
aRegs, samplePos, localProfileBuffer);
});
@ -3434,10 +3435,12 @@ void profiler_suspend_and_sample_thread(int aThreadId, uint32_t aFeatures,
// Suspend, sample, and then resume the target thread.
Sampler sampler(lock);
TimeStamp now = TimeStamp::NowUnfuzzed();
sampler.SuspendAndSampleAndResumeThread(
lock, registeredThread, [&](const Registers& aRegs) {
// The target thread is now suspended. Collect a native backtrace,
// and call the callback.
lock, registeredThread, now,
[&](const Registers& aRegs, const TimeStamp& aNow) {
// The target thread is now suspended. Collect a native
// backtrace, and call the callback.
bool isSynchronous = false;
# if defined(HAVE_FASTINIT_NATIVE_UNWIND)
if (aSampleNative) {
@ -3448,7 +3451,8 @@ void profiler_suspend_and_sample_thread(int aThreadId, uint32_t aFeatures,
DoFramePointerBacktrace(lock, registeredThread, aRegs,
nativeStack);
# elif defined(USE_MOZ_STACK_WALK)
DoMozStackWalkBacktrace(lock, registeredThread, aRegs, nativeStack);
DoMozStackWalkBacktrace(lock, registeredThread, aRegs,
nativeStack);
# else
# error "Invalid configuration"
# endif

View File

@ -45,13 +45,17 @@ size_t RegisteredThread::SizeOfIncludingThis(
return n;
}
void RegisteredThread::GetRunningEventDelay(TimeDuration& aDelay,
void RegisteredThread::GetRunningEventDelay(const TimeStamp& aNow,
TimeDuration& aDelay,
TimeDuration& aRunning) {
if (mThread) { // can be null right at the start of a process
TimeStamp start;
mThread->GetRunningEventDelay(&aDelay, &start);
if (!start.IsNull()) {
aRunning = TimeStamp::Now() - start;
// Note: the timestamp used here will be from when we started to
// suspend and sample the thread; which is also the timestamp
// associated with the sample.
aRunning = aNow - start;
return;
}
}

View File

@ -160,7 +160,8 @@ class RegisteredThread final {
// aRunning is the time the event has been running. If no event is
// running these will both be TimeDuration() (i.e. 0). Both are out
// params, and are always set. Their initial value is discarded.
void GetRunningEventDelay(TimeDuration& aDelay, TimeDuration& aRunning);
void GetRunningEventDelay(const TimeStamp& aNow, TimeDuration& aDelay,
TimeDuration& aRunning);
size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;

View File

@ -279,7 +279,7 @@ void Sampler::Disable(PSLockRef aLock) {
template <typename Func>
void Sampler::SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs) {
const TimeStamp& aNow, const Func& aProcessRegs) {
// Only one sampler thread can be sampling at once. So we expect to have
// complete control over |sSigHandlerCoordinator|.
MOZ_ASSERT(!sSigHandlerCoordinator);
@ -334,7 +334,7 @@ void Sampler::SuspendAndSampleAndResumeThread(
// Extract the current register values.
Registers regs;
PopulateRegsFromContext(regs, &sSigHandlerCoordinator->mUContext);
aProcessRegs(regs);
aProcessRegs(regs, aNow);
//----------------------------------------------------------------//
// Resume the target thread.

View File

@ -76,7 +76,7 @@ void Sampler::Disable(PSLockRef aLock) {}
template <typename Func>
void Sampler::SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs) {
const TimeStamp& aNow, const Func& aProcessRegs) {
thread_act_t samplee_thread =
aRegisteredThread.GetPlatformData()->ProfiledThread();
@ -119,7 +119,7 @@ void Sampler::SuspendAndSampleAndResumeThread(
regs.mFP = reinterpret_cast<Address>(state.REGISTER_FIELD(bp));
regs.mLR = 0;
aProcessRegs(regs);
aProcessRegs(regs, aNow);
}
#undef REGISTER_FIELD

View File

@ -113,7 +113,7 @@ void Sampler::Disable(PSLockRef aLock) {}
template <typename Func>
void Sampler::SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs) {
const TimeStamp& aNow, const Func& aProcessRegs) {
HANDLE profiled_thread =
aRegisteredThread.GetPlatformData()->ProfiledThread();
if (profiled_thread == nullptr) {
@ -159,7 +159,7 @@ void Sampler::SuspendAndSampleAndResumeThread(
Registers regs;
PopulateRegsFromContext(regs, &context);
aProcessRegs(regs);
aProcessRegs(regs, aNow);
//----------------------------------------------------------------//
// Resume the target thread.

View File

@ -2573,13 +2573,17 @@ class Sampler {
// This method suspends and resumes the samplee thread. It calls the passed-in
// function-like object aProcessRegs (passing it a populated |const
// Registers&| arg) while the samplee thread is suspended.
// Registers&| arg) while the samplee thread is suspended. Note that
// the aProcessRegs function must be very careful not to do anything that
// requires a lock, since we may have interrupted the thread at any point.
// As an example, you can't call TimeStamp::Now() since on windows it
// takes a lock on the performance counter.
//
// Func must be a function-like object of type `void()`.
template <typename Func>
void SuspendAndSampleAndResumeThread(
PSLockRef aLock, const RegisteredThread& aRegisteredThread,
const Func& aProcessRegs);
const TimeStamp& aNow, const Func& aProcessRegs);
private:
#if defined(GP_OS_linux) || defined(GP_OS_android)
@ -2868,7 +2872,8 @@ void SamplerThread::Run() {
// Suspend the thread and collect its stack data in the local
// buffer.
mSampler.SuspendAndSampleAndResumeThread(
lock, *registeredThread, [&](const Registers& aRegs) {
lock, *registeredThread, now,
[&](const Registers& aRegs, const TimeStamp& aNow) {
DoPeriodicSample(lock, *registeredThread, *profiledThreadData,
now, aRegs, samplePos, localProfileBuffer);
@ -3046,8 +3051,8 @@ void SamplerThread::Run() {
TimeDuration currentEventDelay;
TimeDuration currentEventRunning;
registeredThread->GetRunningEventDelay(currentEventDelay,
currentEventRunning);
registeredThread->GetRunningEventDelay(
aNow, currentEventDelay, currentEventRunning);
// Note: a different definition of responsiveness than the
// 16ms event injection.
@ -4863,8 +4868,10 @@ void profiler_suspend_and_sample_thread(int aThreadId, uint32_t aFeatures,
// Suspend, sample, and then resume the target thread.
Sampler sampler(lock);
TimeStamp now = TimeStamp::Now();
sampler.SuspendAndSampleAndResumeThread(
lock, registeredThread, [&](const Registers& aRegs) {
lock, registeredThread, now,
[&](const Registers& aRegs, const TimeStamp& aNow) {
// The target thread is now suspended. Collect a native backtrace,
// and call the callback.
bool isSynchronous = false;
@ -4877,7 +4884,8 @@ void profiler_suspend_and_sample_thread(int aThreadId, uint32_t aFeatures,
DoFramePointerBacktrace(lock, registeredThread, aRegs,
nativeStack);
# elif defined(USE_MOZ_STACK_WALK)
DoMozStackWalkBacktrace(lock, registeredThread, aRegs, nativeStack);
DoMozStackWalkBacktrace(lock, registeredThread, aRegs,
nativeStack);
# else
# error "Invalid configuration"
# endif