From 05afe8be308acf10c7aca43dea7cfd66db0ef3c6 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Fri, 25 Aug 2023 14:19:33 -0800 Subject: [PATCH] ref: centralize SentryBinaryImageCache singleton (#3241) --- Sources/Sentry/SentryBinaryImageCache.m | 13 +++---------- Sources/Sentry/SentryCrashStackEntryMapper.m | 8 ++++---- Sources/Sentry/SentryDependencyContainer.m | 13 +++++++++++++ Sources/Sentry/SentrySDK.m | 4 ++-- Sources/Sentry/SentryThreadInspector.m | 3 +-- .../include/HybridPublic/SentryBinaryImageCache.h | 4 ---- .../HybridPublic/SentryDependencyContainer.h | 2 ++ .../Sentry/include/SentryCrashStackEntryMapper.h | 4 +--- Sources/Sentry/include/SentryStacktraceBuilder.h | 1 - Tests/SentryTests/SentryBinaryImageCacheTests.swift | 2 +- .../SentryCrash/SentryBinaryImageCacheTests.m | 3 ++- .../SentryCrashStackEntryMapperTests.swift | 8 ++++---- .../SentryCrash/SentryStacktraceBuilderTests.swift | 2 +- .../SentryCrash/SentryThreadInspectorTests.swift | 4 ++-- .../SentryCrash/TestThreadInspector.swift | 2 +- Tests/SentryTests/SentrySDKTests.swift | 6 +++--- 16 files changed, 40 insertions(+), 39 deletions(-) diff --git a/Sources/Sentry/SentryBinaryImageCache.m b/Sources/Sentry/SentryBinaryImageCache.m index 013e89678db..e98b4374bde 100644 --- a/Sources/Sentry/SentryBinaryImageCache.m +++ b/Sources/Sentry/SentryBinaryImageCache.m @@ -1,5 +1,6 @@ #import "SentryBinaryImageCache.h" #import "SentryCrashBinaryImageCache.h" +#import "SentryDependencyContainer.h" static void binaryImageWasAdded(const SentryCrashBinaryImage *image); @@ -17,14 +18,6 @@ - (void)binaryImageRemoved:(const SentryCrashBinaryImage *)image; @implementation SentryBinaryImageCache -+ (SentryBinaryImageCache *)shared -{ - static SentryBinaryImageCache *instance = nil; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ instance = [[self alloc] init]; }); - return instance; -} - - (void)start { _cache = [NSMutableArray array]; @@ -111,11 +104,11 @@ - (NSInteger)indexOfImage:(uint64_t)address static void binaryImageWasAdded(const SentryCrashBinaryImage *image) { - [SentryBinaryImageCache.shared binaryImageAdded:image]; + [SentryDependencyContainer.sharedInstance.binaryImageCache binaryImageAdded:image]; } static void binaryImageWasRemoved(const SentryCrashBinaryImage *image) { - [SentryBinaryImageCache.shared binaryImageRemoved:image]; + [SentryDependencyContainer.sharedInstance.binaryImageCache binaryImageRemoved:image]; } diff --git a/Sources/Sentry/SentryCrashStackEntryMapper.m b/Sources/Sentry/SentryCrashStackEntryMapper.m index 9fde931c3f0..433ee796995 100644 --- a/Sources/Sentry/SentryCrashStackEntryMapper.m +++ b/Sources/Sentry/SentryCrashStackEntryMapper.m @@ -1,4 +1,6 @@ #import "SentryCrashStackEntryMapper.h" +#import "SentryBinaryImageCache.h" +#import "SentryDependencyContainer.h" #import "SentryFormatter.h" #import "SentryFrame.h" #import "SentryInAppLogic.h" @@ -10,18 +12,15 @@ SentryCrashStackEntryMapper () @property (nonatomic, strong) SentryInAppLogic *inAppLogic; -@property (nonatomic, strong) SentryBinaryImageCache *binaryImageCache; @end @implementation SentryCrashStackEntryMapper - (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic - binaryImageCache:(SentryBinaryImageCache *)binaryImageCache { if (self = [super init]) { self.inAppLogic = inAppLogic; - self.binaryImageCache = binaryImageCache; } return self; } @@ -42,7 +41,8 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack // If there is no symbolication, because debug was disabled // we get image from the cache. if (stackEntry.imageAddress == 0 && stackEntry.imageName == NULL) { - SentryBinaryImageInfo *info = [_binaryImageCache imageByAddress:stackEntry.address]; + SentryBinaryImageInfo *info = [SentryDependencyContainer.sharedInstance.binaryImageCache + imageByAddress:stackEntry.address]; frame.imageAddress = sentry_formatHexAddressUInt64(info.address); frame.package = info.name; diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index 43d06aa24aa..d525f214380 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -1,4 +1,5 @@ #import "SentryANRTracker.h" +#import "SentryBinaryImageCache.h" #import "SentryCurrentDateProvider.h" #import "SentryDispatchFactory.h" #import "SentryDispatchQueueWrapper.h" @@ -139,6 +140,18 @@ - (SentryNSNotificationCenterWrapper *)notificationCenterWrapper return _random; } +- (SentryBinaryImageCache *)binaryImageCache +{ + if (_binaryImageCache == nil) { + @synchronized(sentryDependencyContainerLock) { + if (_binaryImageCache == nil) { + _binaryImageCache = [[SentryBinaryImageCache alloc] init]; + } + } + } + return _binaryImageCache; +} + #if SENTRY_HAS_UIKIT - (SentryScreenshot *)screenshot { diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index da49ce18966..c99a5772001 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -151,7 +151,7 @@ + (void)startWithOptions:(SentryOptions *)options [SentrySDK installIntegrations]; [SentryCrashWrapper.sharedInstance startBinaryImageCache]; - [SentryBinaryImageCache.shared start]; + [SentryDependencyContainer.sharedInstance.binaryImageCache start]; } + (void)startWithConfigureOptions:(void (^)(SentryOptions *options))configureOptions @@ -410,7 +410,7 @@ + (void)close [SentryDependencyContainer reset]; [SentryCrashWrapper.sharedInstance stopBinaryImageCache]; - [SentryBinaryImageCache.shared stop]; + [SentryDependencyContainer.sharedInstance.binaryImageCache stop]; SENTRY_LOG_DEBUG(@"SDK closed!"); } diff --git a/Sources/Sentry/SentryThreadInspector.m b/Sources/Sentry/SentryThreadInspector.m index 9c170770a03..bdbf51b6566 100644 --- a/Sources/Sentry/SentryThreadInspector.m +++ b/Sources/Sentry/SentryThreadInspector.m @@ -69,8 +69,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options [[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes inAppExcludes:options.inAppExcludes]; SentryCrashStackEntryMapper *crashStackEntryMapper = - [[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic - binaryImageCache:SentryBinaryImageCache.shared]; + [[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic]; SentryStacktraceBuilder *stacktraceBuilder = [[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper]; stacktraceBuilder.symbolicate = options.debug; diff --git a/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h b/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h index 35212fd5184..70ea8be25d9 100644 --- a/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h +++ b/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h @@ -1,4 +1,3 @@ -#import "SentryDefines.h" #import NS_ASSUME_NONNULL_BEGIN @@ -15,9 +14,6 @@ NS_ASSUME_NONNULL_BEGIN * performance. */ @interface SentryBinaryImageCache : NSObject -SENTRY_NO_INIT - -@property (nonatomic, readonly, class) SentryBinaryImageCache *shared; - (void)start; diff --git a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h index 217ef814dcf..07da41cabef 100644 --- a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h +++ b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h @@ -2,6 +2,7 @@ @class SentryANRTracker; @class SentryAppStateManager; +@class SentryBinaryImageCache; @class SentryCrashWrapper; @class SentryCurrentDateProvider; @class SentryDebugImageProvider; @@ -54,6 +55,7 @@ SENTRY_NO_INIT @property (nonatomic, strong) SentryDispatchFactory *dispatchFactory; @property (nonatomic, strong) SentryNSTimerFactory *timerFactory; @property (nonatomic, strong) SentryCurrentDateProvider *dateProvider; +@property (nonatomic, strong) SentryBinaryImageCache *binaryImageCache; #if SENTRY_HAS_UIKIT @property (nonatomic, strong) SentryFramesTracker *framesTracker; diff --git a/Sources/Sentry/include/SentryCrashStackEntryMapper.h b/Sources/Sentry/include/SentryCrashStackEntryMapper.h index f6da6109048..f7679c97b4b 100644 --- a/Sources/Sentry/include/SentryCrashStackEntryMapper.h +++ b/Sources/Sentry/include/SentryCrashStackEntryMapper.h @@ -1,4 +1,3 @@ -#import "SentryBinaryImageCache.h" #import "SentryCrashDynamicLinker.h" #import "SentryCrashStackCursor.h" #import "SentryDefines.h" @@ -11,8 +10,7 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryCrashStackEntryMapper : NSObject SENTRY_NO_INIT -- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic - binaryImageCache:(SentryBinaryImageCache *)binaryImageCache; +- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic; /** * Maps the stackEntry of a SentryCrashStackCursor to SentryFrame. diff --git a/Sources/Sentry/include/SentryStacktraceBuilder.h b/Sources/Sentry/include/SentryStacktraceBuilder.h index 77bac993c6b..99f2670e751 100644 --- a/Sources/Sentry/include/SentryStacktraceBuilder.h +++ b/Sources/Sentry/include/SentryStacktraceBuilder.h @@ -1,4 +1,3 @@ -#import "SentryBinaryImageCache.h" #import "SentryCrashMachineContext.h" #import "SentryCrashStackCursor.h" #include "SentryCrashThread.h" diff --git a/Tests/SentryTests/SentryBinaryImageCacheTests.swift b/Tests/SentryTests/SentryBinaryImageCacheTests.swift index 4f3493d9964..a6aa85f7965 100644 --- a/Tests/SentryTests/SentryBinaryImageCacheTests.swift +++ b/Tests/SentryTests/SentryBinaryImageCacheTests.swift @@ -2,7 +2,7 @@ import XCTest class SentryBinaryImageCacheTests: XCTestCase { var sut: SentryBinaryImageCache { - SentryBinaryImageCache.shared + SentryDependencyContainer.sharedInstance().binaryImageCache } override func setUp() { diff --git a/Tests/SentryTests/SentryCrash/SentryBinaryImageCacheTests.m b/Tests/SentryTests/SentryCrash/SentryBinaryImageCacheTests.m index cc2653e623b..b6541cd583c 100644 --- a/Tests/SentryTests/SentryCrash/SentryBinaryImageCacheTests.m +++ b/Tests/SentryTests/SentryCrash/SentryBinaryImageCacheTests.m @@ -1,6 +1,7 @@ #import "SentryBinaryImageCache+Private.h" #import "SentryCrashBinaryImageCache.h" #import "SentryCrashWrapper.h" +#import "SentryDependencyContainer.h" #import #include @@ -262,7 +263,7 @@ - (void)testSentryBinaryImageCacheIntegration { sentrycrashbic_startCache(); - SentryBinaryImageCache *imageCache = SentryBinaryImageCache.shared; + SentryBinaryImageCache *imageCache = SentryDependencyContainer.sharedInstance.binaryImageCache; [imageCache start]; // by calling start, SentryBinaryImageCache will register a callback with // `SentryCrashBinaryImageCache` that should be called for every image already cached. diff --git a/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift b/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift index 7b41d384ab4..d91b1054a86 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift @@ -10,7 +10,7 @@ class SentryCrashStackEntryMapperTests: XCTestCase { override func setUp() { super.setUp() - sut = SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [bundleExecutable], inAppExcludes: []), binaryImageCache: SentryBinaryImageCache.shared) + sut = SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [bundleExecutable], inAppExcludes: [])) } func testSymbolAddress() { @@ -72,8 +72,8 @@ class SentryCrashStackEntryMapperTests: XCTestCase { func testImageFromCache() { var image = createCrashBinaryImage(2_488_998_912) - SentryBinaryImageCache.shared.start() - SentryBinaryImageCache.shared.binaryImageAdded(&image) + SentryDependencyContainer.sharedInstance().binaryImageCache.start() + SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded(&image) var cursor = SentryCrashStackCursor() cursor.stackEntry.address = 2_488_998_950 @@ -83,7 +83,7 @@ class SentryCrashStackEntryMapperTests: XCTestCase { XCTAssertEqual("0x00000000945b1c00", frame.imageAddress ?? "") XCTAssertEqual("Expected Name at 2488998912", frame.package) - SentryBinaryImageCache.shared.stop() + SentryDependencyContainer.sharedInstance().binaryImageCache.stop() } private func getFrameWithImageName(imageName: String) -> Frame { diff --git a/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift b/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift index 28d52a7bdcc..c6ce726207a 100644 --- a/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift @@ -8,7 +8,7 @@ class SentryStacktraceBuilderTests: XCTestCase { let queue = DispatchQueue(label: "SentryStacktraceBuilderTests") var sut: SentryStacktraceBuilder { - let res = SentryStacktraceBuilder(crashStackEntryMapper: SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []), binaryImageCache: SentryBinaryImageCache.shared)) + let res = SentryStacktraceBuilder(crashStackEntryMapper: SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []))) res.symbolicate = true return res } diff --git a/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift b/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift index 79288380354..6c90ee537eb 100644 --- a/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift @@ -5,13 +5,13 @@ class SentryThreadInspectorTests: XCTestCase { private class Fixture { var testMachineContextWrapper = TestMachineContextWrapper() - var stacktraceBuilder = TestSentryStacktraceBuilder(crashStackEntryMapper: SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []), binaryImageCache: SentryBinaryImageCache.shared )) + var stacktraceBuilder = TestSentryStacktraceBuilder(crashStackEntryMapper: SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []))) var keepThreadAlive = true func getSut(testWithRealMachineContextWrapper: Bool = false, symbolicate: Bool = true) -> SentryThreadInspector { let machineContextWrapper = testWithRealMachineContextWrapper ? SentryCrashDefaultMachineContextWrapper() : testMachineContextWrapper as SentryCrashMachineContextWrapper - let stacktraceBuilder = testWithRealMachineContextWrapper ? SentryStacktraceBuilder(crashStackEntryMapper: SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []), binaryImageCache: SentryBinaryImageCache.shared)) : self.stacktraceBuilder + let stacktraceBuilder = testWithRealMachineContextWrapper ? SentryStacktraceBuilder(crashStackEntryMapper: SentryCrashStackEntryMapper(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []))) : self.stacktraceBuilder stacktraceBuilder.symbolicate = symbolicate diff --git a/Tests/SentryTests/SentryCrash/TestThreadInspector.swift b/Tests/SentryTests/SentryCrash/TestThreadInspector.swift index 8eaae4fbfee..984546ed298 100644 --- a/Tests/SentryTests/SentryCrash/TestThreadInspector.swift +++ b/Tests/SentryTests/SentryCrash/TestThreadInspector.swift @@ -7,7 +7,7 @@ class TestThreadInspector: SentryThreadInspector { static var instance: TestThreadInspector { // We need something to pass to the super initializer, because the empty initializer has been marked unavailable. let inAppLogic = SentryInAppLogic(inAppIncludes: [], inAppExcludes: []) - let crashStackEntryMapper = SentryCrashStackEntryMapper(inAppLogic: inAppLogic, binaryImageCache: SentryBinaryImageCache.shared) + let crashStackEntryMapper = SentryCrashStackEntryMapper(inAppLogic: inAppLogic) let stacktraceBuilder = SentryStacktraceBuilder(crashStackEntryMapper: crashStackEntryMapper) return TestThreadInspector(stacktraceBuilder: stacktraceBuilder, andMachineContextWrapper: SentryCrashDefaultMachineContextWrapper()) } diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 4687cd9bc59..f7efdcb02f7 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -125,12 +125,12 @@ class SentrySDKTests: XCTestCase { options.debug = true } - XCTAssertNotNil(SentryBinaryImageCache.shared.cache) - XCTAssertGreaterThan(SentryBinaryImageCache.shared.cache.count, 0) + XCTAssertNotNil(SentryDependencyContainer.sharedInstance().binaryImageCache.cache) + XCTAssertGreaterThan(SentryDependencyContainer.sharedInstance().binaryImageCache.cache.count, 0) SentrySDK.close() - XCTAssertNil(SentryBinaryImageCache.shared.cache) + XCTAssertNil(SentryDependencyContainer.sharedInstance().binaryImageCache.cache) } func testStartWithConfigureOptions_NoDsn() throws {