Skip to content

Commit

Permalink
fix: profiling memory growth fix (#3145)
Browse files Browse the repository at this point in the history
- always ensure the profiler's timeout timer is scheduled on the main thread
- keep references to any aborted profiler until its associated transaction is finished
- fix testConcurrentSpansWithTimeout to properly regression test the scenario
  • Loading branch information
armcknight authored Jul 14, 2023
1 parent 4af7581 commit 4afae53
Show file tree
Hide file tree
Showing 19 changed files with 359 additions and 210 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

- Renamed `enableTimeToFullDisplay` to `enableTimeToFullDisplayTracing` (#3106)
- This is an experimental feature and may change at any time without a major revision.

### Fixes

- Fix potential unbounded memory growth when starting profiled transactions from non-main contexts (#3135)

## 8.9.0-beta.1

Expand Down
1 change: 0 additions & 1 deletion Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -3112,7 +3112,6 @@
8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */,
03F84D1127DD414C008FE43F /* SentryProfiler.h */,
03F84D2B27DD4191008FE43F /* SentryProfiler.mm */,
84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */,
84A888FC28D9B11700C51DFD /* SentryProfiler+Private.h */,
0354A22A2A134D9C003C3A04 /* SentryProfilerState.h */,
84281C642A57D36100EE88F2 /* SentryProfilerState+ObjCpp.h */,
Expand Down
1 change: 1 addition & 0 deletions SentryTestUtils/ClearTestState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class TestCleanup: NSObject {
SentryTracer.resetAppStartMeasurementRead()

#if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)
SentryProfiler.getCurrent().stop(for: .normal)
SentryTracer.resetConcurrencyTracking()
#endif
}
Expand Down
1 change: 1 addition & 0 deletions SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#import "SentryNSTimerFactory.h"
#import "SentryNetworkTracker.h"
#import "SentryPerformanceTracker+Testing.h"
#import "SentryProfiler+Test.h"
#import "SentryRandom.h"
#import "SentrySDK+Private.h"
#import "SentrySDK+Tests.h"
Expand Down
4 changes: 2 additions & 2 deletions Sources/Sentry/SentryFramesTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ - (void)displayLinkCallback
}

# if SENTRY_TARGET_PROFILING_SUPPORTED
if ([SentryProfiler isRunning]) {
if ([SentryProfiler isCurrentlyProfiling]) {
BOOL hasNoFrameRatesYet = self.frameRateTimestamps.count == 0;
uint64_t previousFrameRate
= self.frameRateTimestamps.lastObject[@"value"].unsignedLongLongValue;
Expand Down Expand Up @@ -179,7 +179,7 @@ - (void)reportNewFrame
# if SENTRY_TARGET_PROFILING_SUPPORTED
- (void)recordTimestamp:(uint64_t)timestamp value:(NSNumber *)value array:(NSMutableArray *)array
{
BOOL shouldRecord = [SentryProfiler isRunning];
BOOL shouldRecord = [SentryProfiler isCurrentlyProfiling];
# if defined(TEST) || defined(TESTCI)
shouldRecord = YES;
# endif
Expand Down
6 changes: 3 additions & 3 deletions Sources/Sentry/SentryNSNotificationCenterWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ + (NSNotificationName)willTerminateNotificationName
- (void)addObserver:(id)observer
selector:(SEL)aSelector
name:(NSNotificationName)aName
object:(id)anObject
object:(nullable id)anObject
{
[NSNotificationCenter.defaultCenter addObserver:observer
selector:aSelector
Expand All @@ -67,7 +67,7 @@ - (void)removeObserver:(id)observer name:(NSNotificationName)aName
[NSNotificationCenter.defaultCenter removeObserver:observer name:aName object:nil];
}

- (void)removeObserver:(id)observer name:(NSNotificationName)aName object:(id)anObject
- (void)removeObserver:(id)observer name:(NSNotificationName)aName object:(nullable id)anObject
{
[NSNotificationCenter.defaultCenter removeObserver:observer name:aName object:anObject];
}
Expand All @@ -77,7 +77,7 @@ - (void)removeObserver:(id)observer
[NSNotificationCenter.defaultCenter removeObserver:observer];
}

- (void)postNotificationName:(NSNotificationName)aName object:(id)anObject
- (void)postNotificationName:(NSNotificationName)aName object:(nullable id)anObject
{
[NSNotificationCenter.defaultCenter postNotificationName:aName object:anObject];
}
Expand Down
Loading

0 comments on commit 4afae53

Please sign in to comment.