From e0c18252cac75cfde84318e23110e47a48fa5d6e Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Wed, 13 Dec 2023 10:13:44 +0100 Subject: [PATCH] Discard unfinished spans when the app goes into the background --- BugsnagPerformance.xcodeproj/project.pbxproj | 12 +++ .../Private/AppStateTracker.h | 1 + .../Private/AppStateTracker.m | 4 + .../Private/BugsnagPerformanceImpl.h | 2 + .../Private/BugsnagPerformanceImpl.mm | 14 ++++ Sources/BugsnagPerformance/Private/Tracer.h | 10 +++ Sources/BugsnagPerformance/Private/Tracer.mm | 15 ++++ .../Private/WeakSpansList.h | 74 +++++++++++++++++++ .../Private/WeakSpansList.mm | 24 ++++++ .../WeakSpansListTests.mm | 70 ++++++++++++++++++ 10 files changed, 226 insertions(+) create mode 100644 Sources/BugsnagPerformance/Private/WeakSpansList.h create mode 100644 Sources/BugsnagPerformance/Private/WeakSpansList.mm create mode 100644 Tests/BugsnagPerformanceTests/WeakSpansListTests.mm diff --git a/BugsnagPerformance.xcodeproj/project.pbxproj b/BugsnagPerformance.xcodeproj/project.pbxproj index 414170d4..3e1aef91 100644 --- a/BugsnagPerformance.xcodeproj/project.pbxproj +++ b/BugsnagPerformance.xcodeproj/project.pbxproj @@ -58,6 +58,9 @@ 094FA7522B10EEB600112ED4 /* BugsnagPerformance.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 72E4BB63359EA30E80116E2A /* BugsnagPerformance.framework */; }; 09509B752ADFE9A900A358EC /* TracerTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 09509B742ADFE9A900A358EC /* TracerTests.mm */; }; 096CBC172B1752F700534F0C /* BugsnagPerformanceSwiftUIInstrumentation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 096CBC152B1752F100534F0C /* BugsnagPerformanceSwiftUIInstrumentation.swift */; }; + 0986B7C02B287C9D00BD2CA3 /* WeakSpansList.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0986B7BF2B287C9D00BD2CA3 /* WeakSpansList.mm */; }; + 09B473072B23087D0024CF11 /* WeakSpansList.h in Headers */ = {isa = PBXBuildFile; fileRef = 09B473052B23087D0024CF11 /* WeakSpansList.h */; }; + 09B4730A2B2313570024CF11 /* WeakSpansListTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 09B473092B2313570024CF11 /* WeakSpansListTests.mm */; }; 8A80DA8B2966CE940035BDA9 /* BugsnagPerformance.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 72E4BB63359EA30E80116E2A /* BugsnagPerformance.framework */; }; 8A80DA912966CEB30035BDA9 /* ConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A80DA80296588840035BDA9 /* ConfigurationTests.swift */; }; 962F80F229DB03A400BE82BC /* PerformanceMicrobenchmarkTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 962F80F129DB03A400BE82BC /* PerformanceMicrobenchmarkTests.swift */; }; @@ -272,6 +275,9 @@ 094FA7422B10EDE700112ED4 /* BugsnagPerformanceSwiftUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BugsnagPerformanceSwiftUITests.swift; sourceTree = ""; }; 09509B742ADFE9A900A358EC /* TracerTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = TracerTests.mm; sourceTree = ""; }; 096CBC152B1752F100534F0C /* BugsnagPerformanceSwiftUIInstrumentation.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BugsnagPerformanceSwiftUIInstrumentation.swift; sourceTree = ""; }; + 0986B7BF2B287C9D00BD2CA3 /* WeakSpansList.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WeakSpansList.mm; sourceTree = ""; }; + 09B473052B23087D0024CF11 /* WeakSpansList.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WeakSpansList.h; sourceTree = ""; }; + 09B473092B2313570024CF11 /* WeakSpansListTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WeakSpansListTests.mm; sourceTree = ""; }; 72E4BB63359EA30E80116E2A /* BugsnagPerformance.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = BugsnagPerformance.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 8A80DA80296588840035BDA9 /* ConfigurationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationTests.swift; sourceTree = ""; }; 8A80DA872966CE940035BDA9 /* BugsnagPerformance-SwiftTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "BugsnagPerformance-SwiftTests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -540,6 +546,8 @@ CBF621052919418F004BEE0B /* Uploader.h */, 01EAF97F291AAF19007AC627 /* Utils.h */, 015836C3291264E0002F54C8 /* Version.h */, + 09B473052B23087D0024CF11 /* WeakSpansList.h */, + 0986B7BF2B287C9D00BD2CA3 /* WeakSpansList.mm */, CBE8EA18294B5AB800702950 /* Worker.h */, CBE8EA19294B5AB800702950 /* Worker.mm */, ); @@ -605,6 +613,7 @@ CB2B8A9A2A0924A50054FBBE /* SpanTests.mm */, CB747D20299E5458003CA1B4 /* TimeTests.mm */, 09509B742ADFE9A900A358EC /* TracerTests.mm */, + 09B473092B2313570024CF11 /* WeakSpansListTests.mm */, CB0AD75A295F27DD002A3FB6 /* WorkerTests.mm */, ); path = BugsnagPerformanceTests; @@ -735,6 +744,7 @@ CBE0873329FA984C007455F2 /* BugsnagPerformanceViewType+Private.h in Headers */, CBEC51D62976BCAD009C0CE3 /* Filesystem.h in Headers */, CBEC51C0296DB312009C0CE3 /* JSON.h in Headers */, + 09B473072B23087D0024CF11 /* WeakSpansList.h in Headers */, 0921F02B2A67CBD600C764EB /* BugsnagPerformanceNetworkRequestInfo.h in Headers */, 0122C24A29019770002D243C /* AppStartupInstrumentation.h in Headers */, CB04969B2915194E0097E526 /* OtlpUploader.h in Headers */, @@ -1013,6 +1023,7 @@ CB0AD75B295F27DD002A3FB6 /* WorkerTests.mm in Sources */, CBEC51C3296EC0AC009C0CE3 /* JSONTests.mm in Sources */, 01A414C52912B93F003152A4 /* ResourceAttributesTests.mm in Sources */, + 09B4730A2B2313570024CF11 /* WeakSpansListTests.mm in Sources */, CB2B8A9B2A0924A50054FBBE /* SpanTests.mm in Sources */, CBEC51CE296EEF52009C0CE3 /* PersistenceTests.mm in Sources */, 96D55C882A1EE26A006D1F29 /* SpanStackingHandlerTests.mm in Sources */, @@ -1074,6 +1085,7 @@ CBEC51C1296DB312009C0CE3 /* JSON.mm in Sources */, CBEBE59B29F671A800BF0B4F /* Instrumentation.mm in Sources */, 0122C24029019770002D243C /* SpanData.mm in Sources */, + 0986B7C02B287C9D00BD2CA3 /* WeakSpansList.mm in Sources */, 96D4160C29F276E400AEE435 /* SpanAttributesProvider.mm in Sources */, CB68FABA2A3C27B9005B2CDB /* PersistentDeviceID.mm in Sources */, CBF7C5E2297A8E9100D47719 /* Gzip.m in Sources */, diff --git a/Sources/BugsnagPerformance/Private/AppStateTracker.h b/Sources/BugsnagPerformance/Private/AppStateTracker.h index b99a70d1..fff4abe8 100644 --- a/Sources/BugsnagPerformance/Private/AppStateTracker.h +++ b/Sources/BugsnagPerformance/Private/AppStateTracker.h @@ -15,6 +15,7 @@ NS_ASSUME_NONNULL_BEGIN @property(nonatomic,readonly) BOOL isInForeground; @property(nonatomic,readwrite,strong) void (^onTransitionToForeground)(void); +@property(nonatomic,readwrite,strong) void (^onTransitionToBackground)(void); @end diff --git a/Sources/BugsnagPerformance/Private/AppStateTracker.m b/Sources/BugsnagPerformance/Private/AppStateTracker.m index e474ab8f..ba2c0ce5 100644 --- a/Sources/BugsnagPerformance/Private/AppStateTracker.m +++ b/Sources/BugsnagPerformance/Private/AppStateTracker.m @@ -182,6 +182,10 @@ - (void) handleAppForegroundEvent { - (void) handleAppBackgroundEvent { #pragma clang diagnostic ignored "-Wdirect-ivar-access" _isInForeground = NO; + void (^callback)(void) = self.onTransitionToBackground; + if (callback != nil) { + callback(); + } } @end diff --git a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h index 5e795632..b55612bd 100644 --- a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h +++ b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h @@ -101,6 +101,7 @@ class BugsnagPerformanceImpl: public PhasedStartup { bool sendCurrentBatchTask() noexcept; bool sendRetriesTask() noexcept; bool sendPValueRequestTask() noexcept; + bool sweepTracerTask() noexcept; // Event reactions void onBatchFull() noexcept; @@ -109,6 +110,7 @@ class BugsnagPerformanceImpl: public PhasedStartup { void onFilesystemError() noexcept; void onWorkInterval() noexcept; void onAppEnteredForeground() noexcept; + void onAppEnteredBackground() noexcept; // Utility void wakeWorker() noexcept; diff --git a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm index 19d66207..75cd49fa 100644 --- a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm +++ b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm @@ -146,6 +146,9 @@ appStateTracker_.onTransitionToForeground = ^{ blockThis->onAppEnteredForeground(); }; + appStateTracker_.onTransitionToBackground = ^{ + blockThis->onAppEnteredBackground(); + }; tracer_->start(); @@ -174,6 +177,7 @@ return @[ ^bool() { return blockThis->sendCurrentBatchTask(); }, ^bool() { return blockThis->sendRetriesTask(); }, + ^bool() { return blockThis->sweepTracerTask(); }, ]; } @@ -212,6 +216,12 @@ return false; } +bool BugsnagPerformanceImpl::sweepTracerTask() noexcept { + tracer_->sweep(); + // Never auto-repeat this task, even if work was done; it can wait. + return false; +} + #pragma mark Event Reactions void BugsnagPerformanceImpl::onFilesystemError() noexcept { @@ -258,6 +268,10 @@ wakeWorker(); } +void BugsnagPerformanceImpl::onAppEnteredBackground() noexcept { + tracer_->abortAllOpenSpans(); +} + #pragma mark Utility void BugsnagPerformanceImpl::wakeWorker() noexcept { diff --git a/Sources/BugsnagPerformance/Private/Tracer.h b/Sources/BugsnagPerformance/Private/Tracer.h index eec839fc..564ff144 100644 --- a/Sources/BugsnagPerformance/Private/Tracer.h +++ b/Sources/BugsnagPerformance/Private/Tracer.h @@ -16,6 +16,7 @@ #import "PhasedStartup.h" #import "SpanAttributesProvider.h" #import "SpanStackingHandler.h" +#import "WeakSpansList.h" #import @@ -59,6 +60,11 @@ class Tracer: public PhasedStartup { void cancelQueuedSpan(BugsnagPerformanceSpan *span) noexcept; void onPrewarmPhaseEnded(void) noexcept; + + void abortAllOpenSpans() noexcept; + + // Sweep must be called periodically to avoid a buildup of dead pointers. + void sweep() noexcept; private: Tracer() = delete; @@ -73,6 +79,10 @@ class Tracer: public PhasedStartup { std::mutex prewarmSpansMutex_; NSMutableArray *prewarmSpans_; + // Sloppy list of "open" spans. Some spans may have already been closed, + // but span abort/end are idempotent so it doesn't matter. + std::shared_ptr potentiallyOpenSpans_; + std::shared_ptr batch_; void (^onSpanStarted_)(){ ^(){} }; std::function onViewLoadSpanStarted_{ [](NSString *){} }; diff --git a/Sources/BugsnagPerformance/Private/Tracer.mm b/Sources/BugsnagPerformance/Private/Tracer.mm index f59e8e5c..bf16ecd1 100644 --- a/Sources/BugsnagPerformance/Private/Tracer.mm +++ b/Sources/BugsnagPerformance/Private/Tracer.mm @@ -26,6 +26,7 @@ , sampler_(sampler) , earlyNetworkSpans_([NSMutableArray new]) , prewarmSpans_([NSMutableArray new]) +, potentiallyOpenSpans_(std::make_shared()) , batch_(batch) , onSpanStarted_(onSpanStarted) {} @@ -54,6 +55,19 @@ } } +void +Tracer::abortAllOpenSpans() noexcept { + potentiallyOpenSpans_->abortAll(); +} + +void +Tracer::sweep() noexcept { + constexpr unsigned minEntriesBeforeCompacting = 10000; + if (potentiallyOpenSpans_->count() >= minEntriesBeforeCompacting) { + potentiallyOpenSpans_->compact(); + } +} + BugsnagPerformanceSpan * Tracer::startSpan(NSString *name, SpanOptions options, BSGFirstClass defaultFirstClass) noexcept { __block auto blockThis = this; @@ -84,6 +98,7 @@ spanStackingHandler_->push(span); } [span addAttributes:SpanAttributes::get()]; + potentiallyOpenSpans_->add(span); onSpanStarted_(); return span; } diff --git a/Sources/BugsnagPerformance/Private/WeakSpansList.h b/Sources/BugsnagPerformance/Private/WeakSpansList.h new file mode 100644 index 00000000..c4389211 --- /dev/null +++ b/Sources/BugsnagPerformance/Private/WeakSpansList.h @@ -0,0 +1,74 @@ +// +// WeakSpansList.h +// BugsnagPerformance-iOS +// +// Created by Karl Stenerud on 08.12.23. +// Copyright © 2023 Bugsnag. All rights reserved. +// + +#import +#import + +@interface BSGWeakSpanPointer: NSObject + +// We use a weak wrapper because NSPointerArray.weakObjectsPointerArray's compact method is broken. +@property(nonatomic,readonly,weak) BugsnagPerformanceSpan *span; + +- (instancetype) initWithSpan:(BugsnagPerformanceSpan *)span; + ++ (instancetype) pointerWithSpan:(BugsnagPerformanceSpan *)span; + +@end + +namespace bugsnag { + +class WeakSpansList { +public: + WeakSpansList() + : spans_([NSMutableArray new]) + {} + + void add(BugsnagPerformanceSpan *span) noexcept { + auto ptr = [BSGWeakSpanPointer pointerWithSpan:span]; + std::lock_guard guard(mutex_); + [spans_ addObject:ptr]; + } + + void compact() noexcept { + std::lock_guard guard(mutex_); + bool canCompact = false; + for (BSGWeakSpanPointer *ptr in spans_) { + if (!ptr.span.isValid) { + canCompact = true; + break; + } + } + if (canCompact) { + auto newSpans = [NSMutableArray arrayWithCapacity:spans_.count]; + for (BSGWeakSpanPointer *ptr in spans_) { + if (ptr.span.isValid) { + [newSpans addObject:ptr]; + } + } + spans_ = newSpans; + } + } + + void abortAll() noexcept { + std::lock_guard guard(mutex_); + for (BSGWeakSpanPointer *ptr in spans_) { + [ptr.span abort]; + } + [spans_ removeAllObjects]; + } + + NSUInteger count() noexcept { + return spans_.count; + } + +private: + std::mutex mutex_; + NSMutableArray *spans_; +}; + +} diff --git a/Sources/BugsnagPerformance/Private/WeakSpansList.mm b/Sources/BugsnagPerformance/Private/WeakSpansList.mm new file mode 100644 index 00000000..c537b88d --- /dev/null +++ b/Sources/BugsnagPerformance/Private/WeakSpansList.mm @@ -0,0 +1,24 @@ +// +// WeakSpansList.c +// BugsnagPerformance-iOS +// +// Created by Karl Stenerud on 12.12.23. +// Copyright © 2023 Bugsnag. All rights reserved. +// + +#import "WeakSpansList.h" + +@implementation BSGWeakSpanPointer + +- (instancetype) initWithSpan:(BugsnagPerformanceSpan *)span { + if ((self = [super init])) { + _span = span; + } + return self; +} + ++ (instancetype) pointerWithSpan:(BugsnagPerformanceSpan *)span { + return [[self alloc] initWithSpan:span]; +} + +@end diff --git a/Tests/BugsnagPerformanceTests/WeakSpansListTests.mm b/Tests/BugsnagPerformanceTests/WeakSpansListTests.mm new file mode 100644 index 00000000..f418eea0 --- /dev/null +++ b/Tests/BugsnagPerformanceTests/WeakSpansListTests.mm @@ -0,0 +1,70 @@ +// +// WeakSpansListTests.m +// BugsnagPerformance-iOSTests +// +// Created by Karl Stenerud on 08.12.23. +// Copyright © 2023 Bugsnag. All rights reserved. +// + +#import +#import "WeakSpansList.h" +#import "BugsnagPerformanceSpan+Private.h" +#import "SpanOptions.h" + +using namespace bugsnag; + +@interface WeakSpansListTests : XCTestCase + +@end + +static BugsnagPerformanceSpan *createSpan() { + return [[BugsnagPerformanceSpan alloc] + initWithSpan:std::make_unique(@"test", + IdGenerator::generateTraceId(), + IdGenerator::generateSpanId(), + IdGenerator::generateSpanId(), + SpanOptions().startTime, + BSGFirstClassNo, + ^void(std::shared_ptr) { + })]; +} + +@implementation WeakSpansListTests + +- (void)testAllReleased { + WeakSpansList list; + @autoreleasepool { + list.add(createSpan()); + list.add(createSpan()); + list.add(createSpan()); + } + XCTAssertEqual(3U, list.count()); + list.compact(); + XCTAssertEqual(0U, list.count()); +} + +- (void)testNoneReleased { + WeakSpansList list; + list.add(createSpan()); + list.add(createSpan()); + list.add(createSpan()); + XCTAssertEqual(3U, list.count()); + list.compact(); + XCTAssertEqual(3U, list.count()); +} + +- (void)testSomeReleased { + WeakSpansList list; + list.add(createSpan()); + @autoreleasepool { + list.add(createSpan()); + list.add(createSpan()); + list.add(createSpan()); + } + list.add(createSpan()); + XCTAssertEqual(5U, list.count()); + list.compact(); + XCTAssertEqual(2U, list.count()); +} + +@end