Data race in [SentrySwizzleInfo originalCalled]/[SentrySwizzleInfo setOriginalCalled:]

[hakonk]

Platform

iOS

Environment

Develop

Installed

CocoaPods

Version

8.36.0

Xcode Version

16.0

Did it work on previous versions?

No response

Steps to Reproduce

This happens with Sentry for React Native (v5.24.3). If I understand correctly, this package depends on HybridSDK (v8.36.0), which is part of this code base. Thus, the bug is related to the native Sentry SDK, not the react native package.

Steps to reproduce:

• Enable Thread sanitizer in Xcode
• Build and run with the runtime issue breakpoint enabled and set to "Thread Sanitizer".
• Poke around in the app where interaction leads to network requests via fetch (JS) -> native iOS http implementation.

Expected Result

No breakpoints being hit

Actual Result

tl;dr: As the log shows below, the property originalCalled of SentrySwizzleInfo is read and written to concurrently.

While the write has synchronisation,

SentrySwizzle.m:28-32

#if defined(TEST) || defined(TESTCI) || defined(DEBUG)
    @synchronized(self) {
        self.originalCalled = YES;
    }
#endif // defined(TEST) || defined(TESTCI) || defined(DEBUG)

the read does not:
SentrySwizzle.h:162-167

#if defined(TEST) || defined(TESTCI) || defined(DEBUG)
/**
 * A flag to check whether the original implementation was called.
 */
@property (nonatomic) BOOL originalCalled;
#endif // defined(TEST) || defined(TESTCI) || defined(DEBUG)

As this property only exists for debug builds, it is nonetheless unfortunate when profiling the app in debug. As can be seen in the log below, two threads read and write to the mentioned property when network requests are sent and received via React Native, and while this happens in a React Native app, it really only illustrates that the property is indeed not thread safe for any other use case as well.

WARNING: ThreadSanitizer: data race (pid=57200)
  Write of size 1 at 0x00011ac2c708 by thread T32 (mutexes: write M0):
    #0 -[SentrySwizzleInfo setOriginalCalled:] <null> (App.debug.dylib:arm64+0x1582e30)
    #1 -[SentrySwizzleInfo getOriginalImplementation] <null> (App.debug.dylib:arm64+0x1582bc8)
    #2 __57+[SentryNetworkTrackingIntegration swizzleURLSessionTask]_block_invoke_2.82 <null> (App.debug.dylib:arm64+0x150bc4c)
    #3 -[NSURLSession _onqueue_didCompleteTask:withError:] <null> (CFNetwork:arm64+0x1a6a34)
    #4 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x67b4)

  Previous read of size 1 at 0x00011ac2c708 by thread T30 (mutexes: write M1):
    #0 -[SentrySwizzleInfo originalCalled] <null> (App.debug.dylib:arm64+0x1582da8)
    #1 __57+[SentryNetworkTrackingIntegration swizzleURLSessionTask]_block_invoke_2.82 <null> (App.debug.dylib:arm64+0x150bcb0)
    #2 -[NSURLSessionTask resume] <null> (CFNetwork:arm64+0x218414)
    #3 -[RCTHTTPRequestHandler sendRequest:withDelegate:] <null> (App.debug.dylib:arm64+0x100be40)
    #4 -[RCTNetworkTask start] <null> (App.debug.dylib:arm64+0x101f940)
    #5 -[RCTNetworking sendRequest:responseType:incrementalUpdates:responseSender:] <null> (App.debug.dylib:arm64+0x10186d8)
    #6 __38-[RCTNetworking sendRequest:callback:]_block_invoke_2 <null> (App.debug.dylib:arm64+0x101c4a4)
    #7 __46-[RCTNetworking buildRequest:completionBlock:]_block_invoke_2 <null> (App.debug.dylib:arm64+0x1013b64)
    #8 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x7bccc)
    #9 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x67b4)

  Location is heap block of size 32 at 0x00011ac2c700 allocated by main thread:
    #0 calloc <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x53b90)
    #1 _malloc_type_calloc_outlined <null> (libsystem_malloc.dylib:arm64+0xfe68)
    #2 +[SentrySwizzle swizzleInstanceMethod:inClass:newImpFactory:mode:key:] <null> (App.debug.dylib:arm64+0x1583314)
    #3 +[SentryNetworkTrackingIntegration swizzleURLSessionTask] <null> (App.debug.dylib:arm64+0x150b5d8)
    #4 -[SentryNetworkTrackingIntegration installWithOptions:] <null> (App.debug.dylib:arm64+0x150b1b8)
    #5 +[SentrySDK installIntegrations] <null> (App.debug.dylib:arm64+0x1564074)
    #6 __30+[SentrySDK startWithOptions:]_block_invoke <null> (App.debug.dylib:arm64+0x1561064)
    #7 -[SentryDispatchQueueWrapper dispatchAsyncOnMainQueue:] <null> (App.debug.dylib:arm64+0x14b91d0)
    #8 +[SentrySDK startWithOptions:] <null> (App.debug.dylib:arm64+0x1560968)
    #9 -[RNSentry initNativeSdk:resolve:rejecter:] <null> (App.debug.dylib:arm64+0x63ba88)
    #10 __invoking___ <null> (CoreFoundation:arm64+0x132cbc)
    #11 facebook::react::invokeInner(RCTBridge*, RCTModuleData*, unsigned int, folly::dynamic const&, int, (anonymous namespace)::SchedulingContext) <null> (App.debug.dylib:arm64+0x746f80)
    #12 facebook::react::RCTNativeModule::invoke(unsigned int, folly::dynamic&&, int)::$_0::operator()() const <null> (App.debug.dylib:arm64+0x746434)
    #13 invocation function for block in facebook::react::RCTNativeModule::invoke(unsigned int, folly::dynamic&&, int) <null> (App.debug.dylib:arm64+0x746334)
    #14 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x7bccc)
    #15 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x67b4)
    #16 <null> <null> (0x0001009e5410)

  Mutex M0 (0x00011ac2c700) created at:
    #0 objc_sync_enter <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x79678)
    #1 -[SentrySwizzleInfo getOriginalImplementation] <null> (App.debug.dylib:arm64+0x1582bb4)
    #2 __57+[SentryNetworkTrackingIntegration swizzleURLSessionTask]_block_invoke_2.82 <null> (App.debug.dylib:arm64+0x150bc4c)
    #3 -[__NSCFLocalSessionTask initWithOriginalRequest:ident:taskGroup:] <null> (CFNetwork:arm64+0x1bd60)
    #4 -[SentryQueueableRequestManager addRequest:completionHandler:] <null> (App.debug.dylib:arm64+0x1546698)
    #5 -[SentryHttpTransport sendEnvelope:envelopePath:request:] <null> (App.debug.dylib:arm64+0x14e2d54)
    #6 -[SentryHttpTransport sendAllCachedEnvelopes] <null> (App.debug.dylib:arm64+0x14e264c)
    #7 __36-[SentryHttpTransport sendEnvelope:]_block_invoke <null> (App.debug.dylib:arm64+0x14dfc38)
    #8 __53-[SentryDispatchQueueWrapper dispatchAsyncWithBlock:]_block_invoke <null> (App.debug.dylib:arm64+0x14b90f4)
    #9 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x7bccc)
    #10 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x67b4)

  Mutex M1 (0x000106d4a7d8) created at:
    #0 pthread_mutex_lock <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x311c0)
    #1 std::__1::mutex::lock() <null> (libc++.1.dylib:arm64+0x21178)
    #2 std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:de180100](std::__1::mutex&) <null> (App.debug.dylib:arm64+0x382174)
    #3 -[RCTHTTPRequestHandler sendRequest:withDelegate:] <null> (App.debug.dylib:arm64+0x100b674)
    #4 -[RCTNetworkTask start] <null> (App.debug.dylib:arm64+0x101f940)
    #5 -[RCTNetworking sendRequest:responseType:incrementalUpdates:responseSender:] <null> (App.debug.dylib:arm64+0x10186d8)
    #6 __38-[RCTNetworking sendRequest:callback:]_block_invoke_2 <null> (App.debug.dylib:arm64+0x101c4a4)
    #7 __46-[RCTNetworking buildRequest:completionBlock:]_block_invoke_2 <null> (App.debug.dylib:arm64+0x1013b64)
    #8 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x7bccc)
    #9 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x67b4)

  Thread T32 (tid=13894148, running) is a GCD worker thread

  Thread T30 (tid=13894128, running) is a GCD worker thread

Are you willing to submit a PR?

No response

[hakonk]

Perhaps this can simply be solved by making the property atomic instead of nonatomic?

[brustolin]

Thanks @hakonk for reaching out.

This should be for test only not even debug.

We will provide a fix for this.