fix: Profiling memory leaks (#3055)

This fixes most of the memory leaks reported in #2980 The root of the problem seems to be that `std::thread` does not correctly deallocate parameters that are passed to the newly spawned thread. It's unclear if this is expected behavior or not, but the fix was simple: pass parameters that were being copied (and leaked) by reference. There's also one more unrelated fix to a crash that I found while debugging the leaks - it turns out `-[NSString stringWithUTF8String:]` can return `nil` for malformed data and we weren't checking for nil. --------- Co-authored-by: Sentry Github Bot <[email protected]>
getsentry · May 24, 2023 · 98752f3 · 98752f3
1 parent 2405ba5
commit 98752f3
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Fixes
 
 - Convert one of the two remaining usages of `sprintf` to `snprintf` (#2866)
+- Fix memory leaks in the profiler (#3055)
 
 ## 8.7.2
 

diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm
@@ -354,9 +354,12 @@ - (void)appendBacktrace:(const Backtrace &)backtrace
         }
         if (queueAddress != nil && state.queueMetadata[queueAddress] == nil
             && backtrace.queueMetadata.label != nullptr) {
-            state.queueMetadata[queueAddress] = @ {
-                @"label" : [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()]
-            };
+            NSString *const labelNSStr =
+                [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()];
+            // -[NSString stringWithUTF8String:] can return `nil` for malformed string data
+            if (labelNSStr != nil) {
+                state.queueMetadata[queueAddress] = @ { @"label" : labelNSStr };
+            }
         }
 #    if defined(DEBUG)
         const auto symbols

diff --git a/Sources/Sentry/SentrySamplingProfiler.cpp b/Sources/Sentry/SentrySamplingProfiler.cpp
@@ -24,9 +24,9 @@ namespace profiling {
 
         void *
         samplingThreadMain(mach_port_t port, clock_serv_t clock, mach_timespec_t delaySpec,
-            std::shared_ptr<ThreadMetadataCache> cache,
-            std::function<void(const Backtrace &)> callback, std::atomic_uint64_t &numSamples,
-            std::function<void()> onThreadStart)
+            const std::shared_ptr<ThreadMetadataCache> &cache,
+            const std::function<void(const Backtrace &)> &callback,
+            std::atomic_uint64_t &numSamples, std::function<void()> onThreadStart)
         {
             SENTRY_PROF_LOG_ERROR_RETURN(pthread_setname_np("io.sentry.SamplingProfiler"));
             const int maxSize = 512;
@@ -107,8 +107,8 @@ namespace profiling {
         }
         isSampling_ = true;
         numSamples_ = 0;
-        thread_ = std::thread(samplingThreadMain, port_, clock_, delaySpec_, cache_, callback_,
-            std::ref(numSamples_), onThreadStart);
+        thread_ = std::thread(samplingThreadMain, port_, clock_, delaySpec_, std::cref(cache_),
+            std::cref(callback_), std::ref(numSamples_), onThreadStart);
 
         int policy;
         sched_param param;