Fix timer loop exit condition for sampling profiler

neildhar · facebook-github-bot · commit 212dd9553e66 · 2021-01-27T16:02:13.000-08:00
Summary:
There is a bug in the timer loop implementation. If the profiler is
disabled even before a single iteration of the loop can execute, it
attempts to take a sample instead of exiting immediately.

This has been a source of test flakiness.

Reviewed By: dulinriley

Differential Revision: D26108740

fbshipit-source-id: 9bf706c996288dff233288137cf3f17ffcb64b2f
diff --git a/lib/VM/Profiler/SamplingProfilerPosix.cpp b/lib/VM/Profiler/SamplingProfilerPosix.cpp
@@ -215,23 +215,17 @@ void SamplingProfiler::GlobalProfiler::timerLoop() {
       kMeanMilliseconds, kStdDevMilliseconds};
   std::unique_lock<std::mutex> uniqueLock(profilerLock_);
 
-  while (true) {
+  while (enabled_) {
     if (!sampleStack()) {
       return;
     }
 
     const uint64_t millis = round(std::fabs(distribution(gen)));
     // TODO: make sampling rate configurable.
-    bool disabled = enabledCondVar_.wait_for(
+    enabledCondVar_.wait_for(
         uniqueLock, std::chrono::milliseconds(millis), [this]() {
           return !enabled_;
         });
-    if (disabled) {
-      // The sampling profiler was disabled: don't continue sampling.
-      return;
-    }
-    // Else there was a timeout, which means enabled_ wasn't changed while this
-    // function was waiting. Continue to sample the stack.
   }
 }
 
