Skip to content

Commit

Permalink
ref: centralize SentryBinaryImageCache singleton (#3241)
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight authored Aug 25, 2023
1 parent 9883c0f commit 05afe8b
Show file tree
Hide file tree
Showing 16 changed files with 40 additions and 39 deletions.
13 changes: 3 additions & 10 deletions Sources/Sentry/SentryBinaryImageCache.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import "SentryBinaryImageCache.h"
#import "SentryCrashBinaryImageCache.h"
#import "SentryDependencyContainer.h"

static void binaryImageWasAdded(const SentryCrashBinaryImage *image);

Expand All @@ -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];
Expand Down Expand Up @@ -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];
}
8 changes: 4 additions & 4 deletions Sources/Sentry/SentryCrashStackEntryMapper.m
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#import "SentryCrashStackEntryMapper.h"
#import "SentryBinaryImageCache.h"
#import "SentryDependencyContainer.h"
#import "SentryFormatter.h"
#import "SentryFrame.h"
#import "SentryInAppLogic.h"
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions Sources/Sentry/SentryDependencyContainer.m
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#import "SentryANRTracker.h"
#import "SentryBinaryImageCache.h"
#import "SentryCurrentDateProvider.h"
#import "SentryDispatchFactory.h"
#import "SentryDispatchQueueWrapper.h"
Expand Down Expand Up @@ -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
{
Expand Down
4 changes: 2 additions & 2 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -410,7 +410,7 @@ + (void)close
[SentryDependencyContainer reset];

[SentryCrashWrapper.sharedInstance stopBinaryImageCache];
[SentryBinaryImageCache.shared stop];
[SentryDependencyContainer.sharedInstance.binaryImageCache stop];

SENTRY_LOG_DEBUG(@"SDK closed!");
}
Expand Down
3 changes: 1 addition & 2 deletions Sources/Sentry/SentryThreadInspector.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#import "SentryDefines.h"
#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -15,9 +14,6 @@ NS_ASSUME_NONNULL_BEGIN
* performance.
*/
@interface SentryBinaryImageCache : NSObject
SENTRY_NO_INIT

@property (nonatomic, readonly, class) SentryBinaryImageCache *shared;

- (void)start;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

@class SentryANRTracker;
@class SentryAppStateManager;
@class SentryBinaryImageCache;
@class SentryCrashWrapper;
@class SentryCurrentDateProvider;
@class SentryDebugImageProvider;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions Sources/Sentry/include/SentryCrashStackEntryMapper.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#import "SentryBinaryImageCache.h"
#import "SentryCrashDynamicLinker.h"
#import "SentryCrashStackCursor.h"
#import "SentryDefines.h"
Expand All @@ -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.
Expand Down
1 change: 0 additions & 1 deletion Sources/Sentry/include/SentryStacktraceBuilder.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#import "SentryBinaryImageCache.h"
#import "SentryCrashMachineContext.h"
#import "SentryCrashStackCursor.h"
#include "SentryCrashThread.h"
Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryTests/SentryBinaryImageCacheTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import XCTest

class SentryBinaryImageCacheTests: XCTestCase {
var sut: SentryBinaryImageCache {
SentryBinaryImageCache.shared
SentryDependencyContainer.sharedInstance().binaryImageCache
}

override func setUp() {
Expand Down
3 changes: 2 additions & 1 deletion Tests/SentryTests/SentryCrash/SentryBinaryImageCacheTests.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#import "SentryBinaryImageCache+Private.h"
#import "SentryCrashBinaryImageCache.h"
#import "SentryCrashWrapper.h"
#import "SentryDependencyContainer.h"
#import <XCTest/XCTest.h>

#include <mach-o/dyld.h>
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryTests/SentryCrash/TestThreadInspector.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/SentryTests/SentrySDKTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 05afe8b

Please sign in to comment.