Skip to content

Commit 349b45c

Browse files
huextratantonis
andauthored
fix(ios): synchronize RNSentryTimeToDisplay across main and bridge th… (#5887)
* fix(ios): synchronize RNSentryTimeToDisplay across main and bridge threads putTimeToInitialDisplayForActiveSpan runs on the main thread from SentryFramesTracker/CADisplayLink while setActiveSpanId and pop run from the RN bridge (often off the main thread). Guard static activeSpanId and TTD caches with @synchronized, copy span ids on set, and factor put logic into putTimeToDisplayForLocked to avoid nested @synchronized on the same object. Fixes crashes (EXC_BAD_ACCESS in stringByAppendingString:) from torn reads / use-after-free and unsafe concurrent NSMutableDictionary access. * chore: clang format --------- Co-authored-by: Antonis Lilis <antonis.lilis@sentry.io>
1 parent 3b203e6 commit 349b45c

File tree

2 files changed

+54
-32
lines changed

2 files changed

+54
-32
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66
> make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
77
<!-- prettier-ignore-end -->
88
9+
## Unreleased
10+
11+
### Fixes
12+
13+
- Fix iOS crash (EXC_BAD_ACCESS) in time-to-initial-display when navigating between screens ([#5887](https://github.com/getsentry/sentry-react-native/pull/5887))
14+
915
## 8.6.0
1016

1117
### Fixes

packages/core/ios/RNSentryTimeToDisplay.m

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
#import <QuartzCore/QuartzCore.h>
33
#import <React/RCTLog.h>
44

5+
// All static state below is accessed from the main thread (CADisplayLink, UI) and from the
6+
// React Native bridge / JS thread (setActiveSpanId, pop). Synchronize every access.
57
@implementation RNSentryTimeToDisplay {
68
CADisplayLink *displayLink;
79
RCTResponseSenderBlock resolveBlock;
@@ -13,6 +15,37 @@ @implementation RNSentryTimeToDisplay {
1315

1416
static NSString *activeSpanId;
1517

18+
+ (void)putTimeToDisplayForLocked:(NSString *)screenId value:(NSNumber *)value
19+
{
20+
if (!screenId) {
21+
return;
22+
}
23+
24+
// If key already exists, just update the value,
25+
// this should never happen as TTD is recorded once per navigation
26+
// We avoid updating the age to avoid the age array shift
27+
if ([screenIdToRenderDuration objectForKey:screenId]) {
28+
[screenIdToRenderDuration setObject:value forKey:screenId];
29+
return;
30+
}
31+
32+
// If we haven't reached capacity yet, just append
33+
if (screenIdAge.count < TIME_TO_DISPLAY_ENTRIES_MAX_SIZE) {
34+
[screenIdToRenderDuration setObject:value forKey:screenId];
35+
[screenIdAge addObject:screenId];
36+
} else {
37+
// Remove oldest entry, in most case will already be removed by pop
38+
NSString *oldestKey = screenIdAge[screenIdCurrentIndex];
39+
[screenIdToRenderDuration removeObjectForKey:oldestKey];
40+
41+
[screenIdToRenderDuration setObject:value forKey:screenId];
42+
screenIdAge[screenIdCurrentIndex] = screenId;
43+
44+
// Update circular index, point to the new oldest
45+
screenIdCurrentIndex = (screenIdCurrentIndex + 1) % TIME_TO_DISPLAY_ENTRIES_MAX_SIZE;
46+
}
47+
}
48+
1649
+ (void)initialize
1750
{
1851
if (self == [RNSentryTimeToDisplay class]) {
@@ -27,51 +60,34 @@ + (void)initialize
2760

2861
+ (void)setActiveSpanId:(NSString *)spanId
2962
{
30-
activeSpanId = spanId;
63+
@synchronized([RNSentryTimeToDisplay class]) {
64+
activeSpanId = spanId != nil ? [spanId copy] : nil;
65+
}
3166
}
3267

3368
+ (NSNumber *)popTimeToDisplayFor:(NSString *)screenId
3469
{
35-
NSNumber *value = screenIdToRenderDuration[screenId];
36-
[screenIdToRenderDuration removeObjectForKey:screenId];
37-
return value;
70+
@synchronized([RNSentryTimeToDisplay class]) {
71+
NSNumber *value = screenIdToRenderDuration[screenId];
72+
[screenIdToRenderDuration removeObjectForKey:screenId];
73+
return value;
74+
}
3875
}
3976

4077
+ (void)putTimeToInitialDisplayForActiveSpan:(NSNumber *)value
4178
{
42-
if (activeSpanId != nil) {
43-
NSString *prefixedSpanId = [@"ttid-navigation-" stringByAppendingString:activeSpanId];
44-
[self putTimeToDisplayFor:prefixedSpanId value:value];
79+
@synchronized([RNSentryTimeToDisplay class]) {
80+
if (activeSpanId != nil) {
81+
NSString *prefixedSpanId = [@"ttid-navigation-" stringByAppendingString:activeSpanId];
82+
[self putTimeToDisplayForLocked:prefixedSpanId value:value];
83+
}
4584
}
4685
}
4786

4887
+ (void)putTimeToDisplayFor:(NSString *)screenId value:(NSNumber *)value
4988
{
50-
if (!screenId)
51-
return;
52-
53-
// If key already exists, just update the value,
54-
// this should never happen as TTD is recorded once per navigation
55-
// We avoid updating the age to avoid the age array shift
56-
if ([screenIdToRenderDuration objectForKey:screenId]) {
57-
[screenIdToRenderDuration setObject:value forKey:screenId];
58-
return;
59-
}
60-
61-
// If we haven't reached capacity yet, just append
62-
if (screenIdAge.count < TIME_TO_DISPLAY_ENTRIES_MAX_SIZE) {
63-
[screenIdToRenderDuration setObject:value forKey:screenId];
64-
[screenIdAge addObject:screenId];
65-
} else {
66-
// Remove oldest entry, in most case will already be removed by pop
67-
NSString *oldestKey = screenIdAge[screenIdCurrentIndex];
68-
[screenIdToRenderDuration removeObjectForKey:oldestKey];
69-
70-
[screenIdToRenderDuration setObject:value forKey:screenId];
71-
screenIdAge[screenIdCurrentIndex] = screenId;
72-
73-
// Update circular index, point to the new oldest
74-
screenIdCurrentIndex = (screenIdCurrentIndex + 1) % TIME_TO_DISPLAY_ENTRIES_MAX_SIZE;
89+
@synchronized([RNSentryTimeToDisplay class]) {
90+
[self putTimeToDisplayForLocked:screenId value:value];
7591
}
7692
}
7793

0 commit comments

Comments
 (0)