Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: fix thread sanitizer job #3303

Merged
merged 13 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,6 @@ jobs:
# We don't run all unit tests with Thread Sanitizer enabled because
# that adds a significant overhead.
thread-sanitizer:
# Disabled for now, see https://github.com/getsentry/sentry-cocoa/issues/3200
if: false
name: Unit iOS - Thread Sanitizer
runs-on: macos-13
# When there are threading issues the tests sometimes keep hanging
Expand Down
12 changes: 6 additions & 6 deletions Sources/Sentry/SentryANRTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ - (void)detectANRs
NSInteger reportThreshold = 5;
NSTimeInterval sleepInterval = self.timeoutInterval / reportThreshold;

SentryCurrentDateProvider *dateProvider = SentryDependencyContainer.sharedInstance.dateProvider;

// Canceling the thread can take up to sleepInterval.
while (YES) {
@synchronized(threadLock) {
Expand All @@ -79,8 +81,7 @@ - (void)detectANRs
}
}

NSDate *blockDeadline = [[SentryDependencyContainer.sharedInstance.dateProvider date]
dateByAddingTimeInterval:self.timeoutInterval];
NSDate *blockDeadline = [[dateProvider date] dateByAddingTimeInterval:self.timeoutInterval];

atomic_fetch_add_explicit(&ticksSinceUiUpdate, 1, memory_order_relaxed);

Expand All @@ -107,8 +108,7 @@ - (void)detectANRs
// an ANR. If the app gets suspended this thread could sleep and wake up again. To avoid
// false positives, we don't report ANRs if the delta is too big.
NSTimeInterval deltaFromNowToBlockDeadline =
[[SentryDependencyContainer.sharedInstance.dateProvider date]
timeIntervalSinceDate:blockDeadline];
[[dateProvider date] timeIntervalSinceDate:blockDeadline];

if (deltaFromNowToBlockDeadline >= self.timeoutInterval) {
SENTRY_LOG_DEBUG(
Expand Down Expand Up @@ -165,8 +165,8 @@ - (void)addListener:(id<SentryANRTrackerDelegate>)listener
@synchronized(self.listeners) {
[self.listeners addObject:listener];

if (self.listeners.count > 0 && state == kSentryANRTrackerNotRunning) {
@synchronized(threadLock) {
@synchronized(threadLock) {
if (self.listeners.count > 0 && state == kSentryANRTrackerNotRunning) {
if (state == kSentryANRTrackerNotRunning) {
state = kSentryANRTrackerStarting;
[NSThread detachNewThreadSelector:@selector(detectANRs)
Expand Down
4 changes: 2 additions & 2 deletions Sources/Sentry/SentryFileManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,10 @@ - (void)removeFileAtPath:(NSString *)path
- (NSString *)storeEnvelope:(SentryEnvelope *)envelope
{
NSData *envelopeData = [SentrySerialization dataWithEnvelope:envelope error:nil];
NSString *path =
[self.envelopesPath stringByAppendingPathComponent:[self uniqueAscendingJsonName]];

@synchronized(self) {
NSString *path =
[self.envelopesPath stringByAppendingPathComponent:[self uniqueAscendingJsonName]];
SENTRY_LOG_DEBUG(@"Writing envelope to path: %@", path);

if (![self writeData:envelopeData toPath:path]) {
Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/SentryFramesTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ - (instancetype)initWithDisplayLinkWrapper:(SentryDisplayLinkWrapper *)displayLi
_displayLinkWrapper = displayLinkWrapper;
_listeners = [NSHashTable weakObjectsHashTable];
[self resetFrames];
SENTRY_LOG_DEBUG(@"Initialized frame tracker %@", self);
}
return self;
}
Expand Down Expand Up @@ -146,7 +147,8 @@ - (void)displayLinkCallback
&& frameDuration <= SentryFrozenFrameThreshold) {
_slowFrames++;
# if SENTRY_TARGET_PROFILING_SUPPORTED
SENTRY_LOG_DEBUG(@"Capturing slow frame starting at %llu.", thisFrameSystemTimestamp);
SENTRY_LOG_DEBUG(@"Capturing slow frame starting at %llu (frame tracker: %@).",
thisFrameSystemTimestamp, self);
[self recordTimestamp:thisFrameSystemTimestamp
value:@(thisFrameSystemTimestamp - self.previousFrameSystemTimestamp)
array:self.slowFrameTimestamps];
Expand Down
16 changes: 13 additions & 3 deletions Sources/Sentry/SentryLog.m
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,19 @@ @implementation SentryLog
static BOOL isDebug = YES;
static SentryLevel diagnosticLevel = kSentryLevelError;
static SentryLogOutput *logOutput;
static NSObject *logConfigureLock;

+ (void)initialize
{
logConfigureLock = [[NSObject alloc] init];
}

+ (void)configure:(BOOL)debug diagnosticLevel:(SentryLevel)level
{
isDebug = debug;
diagnosticLevel = level;
@synchronized(logConfigureLock) {
isDebug = debug;
diagnosticLevel = level;
}
}

+ (void)logWithMessage:(NSString *)message andLevel:(SentryLevel)level
Expand All @@ -33,7 +41,9 @@ + (void)logWithMessage:(NSString *)message andLevel:(SentryLevel)level

+ (BOOL)willLogAtLevel:(SentryLevel)level
{
return isDebug && level != kSentryLevelNone && level >= diagnosticLevel;
@synchronized(logConfigureLock) {
return isDebug && level != kSentryLevelNone && level >= diagnosticLevel;
}
}

// Internal and only needed for testing.
Expand Down
10 changes: 6 additions & 4 deletions Sources/Sentry/SentryNetworkTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,12 @@ - (void)urlSessionTask:(NSURLSessionTask *)sessionTask setState:(NSURLSessionTas

- (void)captureFailedRequests:(NSURLSessionTask *)sessionTask
{
if (!self.isCaptureFailedRequestsEnabled) {
SENTRY_LOG_DEBUG(
@"captureFailedRequestsEnabled is disabled, not capturing HTTP Client errors.");
return;
@synchronized(self) {
if (!self.isCaptureFailedRequestsEnabled) {
SENTRY_LOG_DEBUG(
@"captureFailedRequestsEnabled is disabled, not capturing HTTP Client errors.");
return;
}
}

// if request or response are null, we can't raise the event
Expand Down
29 changes: 18 additions & 11 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,20 @@
std::mutex _gProfilerLock;
SentryProfiler *_Nullable _gCurrentProfiler;

BOOL
threadSanitizerIsPresent(void)
{
# if defined(__has_feature)
# if __has_feature(thread_sanitizer)
return YES;
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wunreachable-code"
# endif // __has_feature(thread_sanitizer)
# endif // defined(__has_feature)

return NO;
}

NSString *
profilerTruncationReasonName(SentryProfilerTruncationReason reason)
{
Expand Down Expand Up @@ -497,17 +511,10 @@

- (void)start
{
// Disable profiling when running with TSAN because it produces a TSAN false
// positive, similar to the situation described here:
// https://github.com/envoyproxy/envoy/issues/2561
# if defined(__has_feature)
# if __has_feature(thread_sanitizer)
SENTRY_LOG_DEBUG(@"Disabling profiling when running with TSAN");
return;
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wunreachable-code"
# endif // __has_feature(thread_sanitizer)
# endif // defined(__has_feature)
if (threadSanitizerIsPresent()) {
SENTRY_LOG_DEBUG(@"Disabling profiling when running with TSAN");
return;

Check warning on line 516 in Sources/Sentry/SentryProfiler.mm

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryProfiler.mm#L516

Added line #L516 was not covered by tests
}

if (_profiler != nullptr) {
// This theoretically shouldn't be possible as long as we're checking for nil and running
Expand Down
29 changes: 17 additions & 12 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,29 @@ + (void)setStartInvocations:(NSUInteger)value

+ (void)startWithOptions:(SentryOptions *)options
{
[SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel];

// We accept the tradeoff that the SDK might not be fully initialized directly after
// initializing it on a background thread because scheduling the init synchronously on the main
// thread could lead to deadlocks.
Comment on lines 142 to 144
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: I think that comment should stay directly above [SentryThreadWrapper onMainThread:^{

[SentryThreadWrapper onMainThread:^{
startInvocations++;
SENTRY_LOG_DEBUG(@"Starting SDK...");

[SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel];
startInvocations++;

SentryClient *newClient = [[SentryClient alloc] initWithOptions:options];
[newClient.fileManager moveAppStateToPreviousAppState];
[newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs];
SentryClient *newClient = [[SentryClient alloc] initWithOptions:options];
[newClient.fileManager moveAppStateToPreviousAppState];
[newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs];

SentryScope *scope = options.initialScope(
[[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]);
// The Hub needs to be initialized with a client so that closing a session
// can happen.
[SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]];
SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);
SentryScope *scope
= options.initialScope([[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]);
// The Hub needs to be initialized with a client so that closing a session
// can happen.
[SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]];
SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);

SENTRY_LOG_DEBUG(@"Dispatching init work required to run on main thread.");
[SentryThreadWrapper onMainThread:^{
SENTRY_LOG_DEBUG(@"SDK main thread init started...");
[SentrySDK installIntegrations];

[SentryCrashWrapper.sharedInstance startBinaryImageCache];
Expand Down
10 changes: 8 additions & 2 deletions Sources/Sentry/SentrySpan.m
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
@implementation SentrySpan {
NSMutableDictionary<NSString *, id> *_data;
NSMutableDictionary<NSString *, id> *_tags;
NSObject *_stateLock;
BOOL _isFinished;
}

Expand All @@ -34,6 +35,7 @@ - (instancetype)initWithContext:(SentrySpanContext *)context
self.startTimestamp = [SentryDependencyContainer.sharedInstance.dateProvider date];
_data = [[NSMutableDictionary alloc] init];
_tags = [[NSMutableDictionary alloc] init];
_stateLock = [[NSObject alloc] init];
_isFinished = NO;

_status = kSentrySpanStatusUndefined;
Expand Down Expand Up @@ -133,7 +135,9 @@ - (void)setMeasurement:(NSString *)name value:(NSNumber *)value unit:(SentryMeas

- (BOOL)isFinished
{
return _isFinished;
@synchronized(_stateLock) {
return _isFinished;
}
}

- (void)finish
Expand All @@ -145,7 +149,9 @@ - (void)finish
- (void)finishWithStatus:(SentrySpanStatus)status
{
self.status = status;
_isFinished = YES;
@synchronized(_stateLock) {
_isFinished = YES;
}
if (self.timestamp == nil) {
self.timestamp = [SentryDependencyContainer.sharedInstance.dateProvider date];
SENTRY_LOG_DEBUG(@"Setting span timestamp: %@ at system time %llu", self.timestamp,
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/SentryStacktraceBuilder.m
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#import "SentryCrashSymbolicator.h"
#import "SentryFrame.h"
#import "SentryFrameRemover.h"
#import "SentryLog.h"
#import "SentryStacktrace.h"
#import <dlfcn.h>

Expand Down Expand Up @@ -106,6 +107,7 @@ - (SentryStacktrace *)buildStacktraceForCurrentThread

- (nullable SentryStacktrace *)buildStacktraceForCurrentThreadAsyncUnsafe
{
SENTRY_LOG_DEBUG(@"Building async-unsafe stack trace...");
SentryCrashStackCursor stackCursor;
sentrycrashsc_initSelfThread(&stackCursor, 0);
stackCursor.symbolicate = sentrycrashsymbolicator_symbolicate_async_unsafe;
Expand Down
3 changes: 3 additions & 0 deletions Sources/Sentry/SentryThreadWrapper.m
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#import "SentryThreadWrapper.h"
#import "SentryLog.h"

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -22,8 +23,10 @@ - (void)threadFinished:(NSUUID *)threadID
+ (void)onMainThread:(void (^)(void))block
{
if ([NSThread isMainThread]) {
SENTRY_LOG_DEBUG(@"Already on main thread.");
block();
} else {
SENTRY_LOG_DEBUG(@"Dispatching asynchronously to main queue.");
dispatch_async(dispatch_get_main_queue(), block);
}
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/Sentry/include/SentryProfiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ SENTRY_EXTERN NSString *const kSentryProfilerSerializationKeyFrameRates;

SENTRY_EXTERN_C_BEGIN

/**
* Disable profiling when running with TSAN because it produces a TSAN false positive, similar to
* the situation described here: https://github.com/envoyproxy/envoy/issues/2561
*/
BOOL threadSanitizerIsPresent(void);

NSString *profilerTruncationReasonName(SentryProfilerTruncationReason reason);

SENTRY_EXTERN_C_END
Expand Down
3 changes: 3 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,12 @@ static sentrycrashbic_cacheChangeCallback imageRemovedCallback = NULL;
static void
binaryImageAdded(const struct mach_header *header, intptr_t slide)
{
pthread_mutex_lock(&binaryImagesMutex);
if (tailNode == NULL) {
pthread_mutex_unlock(&binaryImagesMutex);
return;
}
pthread_mutex_unlock(&binaryImagesMutex);
Dl_info info;
if (!dladdr(header, &info) || info.dli_fname == NULL) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "SentryCrashStackCursor_SelfThread.h"
#include "SentryCrashStackCursor_Backtrace.h"
#import "SentryLog.h"
#include <execinfo.h>

// #define SentryCrashLogger_LocalLevel TRACE
Expand Down Expand Up @@ -57,17 +58,23 @@
int backtraceLength;
if (@available(macOS 12.0, iOS 15.0, tvOS 15.0, watchOS 8.0, *)) {
if (stitchSwiftAsync) {
SENTRY_LOG_DEBUG(@"Retrieving backtrace with async swift stitching...");
backtraceLength
= (int)backtrace_async((void **)context->backtrace, MAX_BACKTRACE_LENGTH, NULL);
} else {
SENTRY_LOG_DEBUG(@"Retrieving backtrace without async swift stitching...");
backtraceLength = backtrace((void **)context->backtrace, MAX_BACKTRACE_LENGTH);
}
} else {
SENTRY_LOG_DEBUG(
@"Retrieving backtrace without async swift stitching (old platform versions)...");
backtraceLength = backtrace((void **)context->backtrace, MAX_BACKTRACE_LENGTH);
}
#else
SENTRY_LOG_DEBUG(@"Retrieving backtrace without async swift stitching (old Xcode versions)...");
int backtraceLength = backtrace((void **)context->backtrace, MAX_BACKTRACE_LENGTH);
#endif

SENTRY_LOG_DEBUG(@"Finished retrieving backtrace.");
sentrycrashsc_initWithBacktrace(cursor, context->backtrace, backtraceLength, skipEntries + 1);
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@

#if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)
func assertProfilingData(slow: UInt? = nil, frozen: UInt? = nil, frameRates: UInt? = nil) throws {
if threadSanitizerIsPresent() {
// profiling data will not have been gathered with TSAN running
return

Check warning on line 179 in Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift

View check run for this annotation

Codecov / codecov/patch

Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift#L178-L179

Added lines #L178 - L179 were not covered by tests
}

func assertFrameInfo(frame: [String: NSNumber]) throws {
XCTAssertNotNil(frame["timestamp"], "Expected a timestamp for the frame.")
XCTAssertNotNil(frame["value"], "Expected a duration value for the frame.")
Expand Down
3 changes: 3 additions & 0 deletions Tests/SentryTests/PrivateSentrySDKOnlyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@
* Smoke Tests profiling via PrivateSentrySDKOnly. Actual profiling unit tests are done elsewhere.
*/
func testProfilingStartAndCollect() throws {
if threadSanitizerIsPresent() {
throw XCTSkip("Profiler does not run if thread sanitizer is attached.")

Check warning on line 129 in Tests/SentryTests/PrivateSentrySDKOnlyTests.swift

View check run for this annotation

Codecov / codecov/patch

Tests/SentryTests/PrivateSentrySDKOnlyTests.swift#L129

Added line #L129 was not covered by tests
}
let options = Options()
options.dsn = TestConstants.dsnAsString(username: "SentryFramesTrackingIntegrationTests")
let client = TestClient(options: options)
Expand Down
19 changes: 18 additions & 1 deletion Tests/SentryTests/SentryCrash/SentryCrashCPU_Tests.m
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,26 @@ @implementation SentryCrashCPU_Tests

- (void)testCPUState
{
NSObject *notificationObject = [[NSObject alloc] init];
TestThread *thread = [[TestThread alloc] init];
thread.notificationObject = notificationObject;

XCTestExpectation *exp = [self expectationWithDescription:@"thread started"];
[NSNotificationCenter.defaultCenter
addObserverForName:@"io.sentry.test.TestThread.main"
object:notificationObject
queue:nil
usingBlock:^(NSNotification *_Nonnull __unused notification) {
[NSNotificationCenter.defaultCenter
removeObserver:self
name:@"io.sentry.test.TestThread.main"
object:notificationObject];
[exp fulfill];
}];

[thread start];
[NSThread sleepForTimeInterval:0.1];
[self waitForExpectationsWithTimeout:1 handler:nil];

kern_return_t kr;
kr = thread_suspend(thread.thread);
XCTAssertTrue(kr == KERN_SUCCESS, @"");
Expand Down
Loading
Loading