From 73bd871c4c31a4c402415680b758962b59496fff Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 22 Nov 2023 15:16:32 -0900 Subject: [PATCH] fix: check for null value returned from pthread_mach_thread_np --- Sources/Sentry/PrivateSentrySDKOnly.mm | 9 +++++---- Sources/Sentry/SentryProfiler.mm | 6 +++++- Sources/Sentry/SentryThreadHandle.cpp | 3 +++ Sources/Sentry/SentryTransactionContext.mm | 18 +++++++++++++----- .../include/SentryTransactionContext+Private.h | 11 +++++++---- .../SentryProfilerSwiftTests.swift | 2 +- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/Sources/Sentry/PrivateSentrySDKOnly.mm b/Sources/Sentry/PrivateSentrySDKOnly.mm index 6b5ab722986..bc25ebc8c4b 100644 --- a/Sources/Sentry/PrivateSentrySDKOnly.mm +++ b/Sources/Sentry/PrivateSentrySDKOnly.mm @@ -141,10 +141,11 @@ + (uint64_t)startProfilerForTrace:(SentryId *)traceId; if (payload != nil) { payload[@"platform"] = SentryPlatformName; - payload[@"transaction"] = @{ - @"active_thread_id" : - [NSNumber numberWithLongLong:sentry::profiling::ThreadHandle::current()->tid()] - }; + const auto thread = sentry::profiling::ThreadHandle::current(); + if (thread != nullptr) { + payload[@"transaction"] = + @{ @"active_thread_id" : [NSNumber numberWithLongLong:thread->tid()] }; + } } return payload; diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index c27554a563c..e02c53af785 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -428,12 +428,16 @@ + (void)updateProfilePayload:(NSMutableDictionary *)payload forTransaction:(SentryTransaction *)transaction; { payload[@"platform"] = transaction.platform; + + const auto activeThreadID = + [transaction.trace.transactionContext sentry_threadInfo].threadId ?: @(-1); payload[@"transaction"] = @{ @"id" : transaction.eventId.sentryIdString, @"trace_id" : transaction.trace.traceId.sentryIdString, @"name" : transaction.transaction, - @"active_thread_id" : [transaction.trace.transactionContext sentry_threadInfo].threadId + @"active_thread_id" : activeThreadID }; + const auto timestamp = transaction.trace.originalStartTimestamp; if (UNLIKELY(timestamp == nil)) { SENTRY_LOG_WARN(@"There was no start timestamp on the provided transaction. Falling back " diff --git a/Sources/Sentry/SentryThreadHandle.cpp b/Sources/Sentry/SentryThreadHandle.cpp index 2aa44ac6530..d53e7d32f8f 100644 --- a/Sources/Sentry/SentryThreadHandle.cpp +++ b/Sources/Sentry/SentryThreadHandle.cpp @@ -45,6 +45,9 @@ namespace profiling { ThreadHandle::current() noexcept { const auto thread = pthread_mach_thread_np(pthread_self()); + if (thread == (mach_port_t)NULL) { + return nullptr; + } return std::make_unique(thread); } diff --git a/Sources/Sentry/SentryTransactionContext.mm b/Sources/Sentry/SentryTransactionContext.mm index d0b94b9fe66..c62e2b3c7cf 100644 --- a/Sources/Sentry/SentryTransactionContext.mm +++ b/Sources/Sentry/SentryTransactionContext.mm @@ -116,25 +116,31 @@ - (instancetype)initWithName:(NSString *)name _name = [NSString stringWithString:name]; _nameSource = source; self.parentSampled = parentSampled; +#if SENTRY_TARGET_PROFILING_SUPPORTED [self getThreadInfo]; +#endif // SENTRY_TARGET_PROFILING_SUPPORTED } return self; } +#if SENTRY_TARGET_PROFILING_SUPPORTED - (void)getThreadInfo { -#if SENTRY_TARGET_PROFILING_SUPPORTED - const auto threadID = sentry::profiling::ThreadHandle::current()->tid(); + const auto thread = sentry::profiling::ThreadHandle::current(); + if (thread == nullptr) { + return; + } + const auto threadID = thread->tid(); self.threadInfo = [[SentryThread alloc] initWithThreadId:@(threadID)]; -#endif } +#endif // SENTRY_TARGET_PROFILING_SUPPORTED #if SENTRY_TARGET_PROFILING_SUPPORTED -- (SentryThread *)sentry_threadInfo +- (nullable SentryThread *)sentry_threadInfo { return self.threadInfo; } -#endif +#endif // SENTRY_TARGET_PROFILING_SUPPORTED - (void)commonInitWithName:(NSString *)name source:(SentryTransactionNameSource)source @@ -143,7 +149,9 @@ - (void)commonInitWithName:(NSString *)name _name = [NSString stringWithString:name]; _nameSource = source; self.parentSampled = parentSampled; +#if SENTRY_TARGET_PROFILING_SUPPORTED [self getThreadInfo]; +#endif // SENTRY_TARGET_PROFILING_SUPPORTED SENTRY_LOG_DEBUG(@"Created transaction context with name %@", name); } diff --git a/Sources/Sentry/include/SentryTransactionContext+Private.h b/Sources/Sentry/include/SentryTransactionContext+Private.h index 92a572c5dfb..b3106ba2f2f 100644 --- a/Sources/Sentry/include/SentryTransactionContext+Private.h +++ b/Sources/Sentry/include/SentryTransactionContext+Private.h @@ -37,12 +37,15 @@ SentryTransactionContext () parentSampled:(SentrySampleDecision)parentSampled; #if SENTRY_TARGET_PROFILING_SUPPORTED + // This is currently only exposed for testing purposes, see -[SentryProfilerTests -// testProfilerMutationDuringSerialization] -@property (nonatomic, strong) SentryThread *threadInfo; +// testProfilerMutationDuringSerialization]. Can be null if there was a problem +// getting the current thread info from the underlying API. +@property (nonatomic, strong, nullable) SentryThread *threadInfo; + +- (nullable SentryThread *)sentry_threadInfo; -- (SentryThread *)sentry_threadInfo; -#endif +#endif // SENTRY_TARGET_PROFILING_SUPPORTED @end diff --git a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift index a11007012ee..caea514c8f7 100644 --- a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift +++ b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift @@ -826,7 +826,7 @@ private extension SentryProfilerSwiftTests { XCTAssertNotEqual(SentryId.empty, linkedTransactionTraceId) let activeThreadId = try XCTUnwrap(linkedTransactionInfo["active_thread_id"] as? NSNumber) - XCTAssertEqual(activeThreadId, latestTransaction.trace.transactionContext.sentry_threadInfo().threadId) + XCTAssertEqual(activeThreadId, try XCTUnwrap(latestTransaction.trace.transactionContext.sentry_threadInfo()).threadId) for sample in samples { let timestamp = try XCTUnwrap(sample["elapsed_since_start_ns"] as? NSString)