fix: Collect samples for idle threads in iOS profiler (#1978)

Remove the check that skips collecting a sample for an idle thread.

We were seeing an issue where we found function frames in profiles that were taking way longer than they should, because of missing samples in between.

That's because we were intentionally skipping collecting samples when the thread was idle (which is frequently the case). Not collecting a sample when the thread is idle means that we will have irregular sample timestamps, which cause an incorrect calculation of the function durations.

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Fixes
 
 - Remove Sentry keys from cached HTTP request headers (#1975)
+- Collect samples for idle threads in iOS profiler (#1978)
 
 ## 7.21.0
 

Sources/Sentry/SentryBacktrace.cpp

@@ -107,9 +107,6 @@ namespace profiling {
     {
         const auto pair = ThreadHandle::allExcludingCurrent();
         for (const auto &thread : pair.first) {
-            if (thread->isIdle()) {
-                continue;
-            }
             Backtrace bt;
             auto metadata = cache->metadataForThread(*thread);
             if (metadata.threadID == 0) {

Tests/SentryTests/Profiling/SentryBacktraceTests.mm

@@ -109,7 +109,7 @@
 }
 
 void *
-threadEntry(__unused void *ptr)
+threadEntry(void *ptr)
 {
     if (pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, nullptr) != 0) {
         return nullptr;

Tests/SentryTests/Profiling/SentrySamplingProfilerTests.mm

@@ -25,8 +25,19 @@ - (void)testProfiling
 {
     const auto cache = std::make_shared<ThreadMetadataCache>();
     const std::uint32_t samplingRateHz = 300;
-    const auto profiler
-        = std::make_shared<SamplingProfiler>([](__unused auto backtrace) {}, samplingRateHz);
+
+    pthread_t idleThread;
+    XCTAssertEqual(pthread_create(&idleThread, nullptr, idleThreadEntry, nullptr), 0);
+    int numIdleSamples = 0;
+
+    const auto profiler = std::make_shared<SamplingProfiler>(
+        [&](auto &backtrace) {
+            const auto thread = backtrace.threadMetadata.threadID;
+            if (thread == pthread_mach_thread_np(idleThread)) {
+                numIdleSamples++;
+            }
+        },
+        samplingRateHz);
     XCTAssertFalse(profiler->isSampling());
 
     std::uint64_t start = 0;
@@ -42,6 +53,22 @@ - (void)testProfiling
     XCTAssertGreaterThan(start, static_cast<std::uint64_t>(0));
     XCTAssertGreaterThan(std::chrono::duration_cast<std::chrono::seconds>(duration).count(), 0);
     XCTAssertGreaterThan(profiler->numSamples(), static_cast<std::uint64_t>(0));
+    XCTAssertGreaterThan(numIdleSamples, 0);
+}
+
+static void *
+idleThreadEntry(__unused void *ptr)
+{
+    // Wait on a condition variable that will never be signaled to make the thread idle.
+    pthread_cond_t cv;
+    pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+    if (pthread_cond_init(&cv, NULL) != 0) {
+        return nullptr;
+    }
+    if (pthread_cond_wait(&cv, &mutex) != 0) {
+        return nullptr;
+    }
+    return nullptr;
 }
 
 @end