Skip to content

Commit

Permalink
fix: Profiling memory leaks (#3055)
Browse files Browse the repository at this point in the history
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 <bot+github-bot@sentry.io>
  • Loading branch information
indragiek and getsentry-bot authored May 24, 2023
1 parent 2405ba5 commit 98752f3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 6 additions & 3 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions Sources/Sentry/SentrySamplingProfiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 98752f3

Please sign in to comment.