Skip to content

Commit 85ceff5

Browse files
Saadnajmifacebook-github-bot
authored andcommitted
Port RCTDisplayLink fix from React Native macOS (#35359)
Summary: Cherry pick microsoft@e877ebf from React Native macOS into React Native Core. We've had this bug fix in our fork for a while now, so it's probably safe. === Original change notes === First, misc straightforward deprecated API updates. Second, defensive changes to potentially address RCTDisplayLink crash. Real-world data suggests crashes with this stack: CVDisplayLink::start() -[RCTDisplayLink updateJSDisplayLinkState] (RCTDisplayLink.m:157) -[RCTDisplayLink registerModuleForFrameUpdates:withModuleData:]_block_invoke (RCTDisplayLink.m:67) -[RCTTiming timerDidFire] (RCTTiming.mm:324) -[_RCTTimingProxy timerDidFire] (RCTTiming.mm:93) Some symbols are missing in this stack, presumably due to compiler optimizations. -updateJSDisplayLinkState is calling CVDisplayLinkStart as a result of a call to "_jsDisplayLink.paused = NO". -registerModuleForFrameUpdates block is presumably getting called via pauseCallback, likely via [RCTTiming startTimers], presumably owned by RCTCxxBridge. The most likely immediate explanation for the crash is that we are calling CVDisplayLinkStart with a zombie _displayLink pointer. However there is a lot of indirection here as well as thread-hopping, and unfortunately no clearly incorrect code that would explain such a zombie pointer. Some defensive changes: -explicitly remove the association to pauseCallback when underlying display link object is invalidated. -remove a prior attempt at additional check in updateJSDisplayLinkState itself as it is not relevant. -make sure we explicitly set _displayLink to NULL when we release it, such that there is one less failure point. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - RCTDisplayLink fix plus bonus deprecated API cleanup Pull Request resolved: #35359 Test Plan: CI should pass. Reviewed By: cipolleschi Differential Revision: D41335438 Pulled By: christophpurrer fbshipit-source-id: 5e97d28eab7dd8e5c81d0a5386df6631e16405a2
1 parent 50f7804 commit 85ceff5

File tree

1 file changed

+7
-0
lines changed

1 file changed

+7
-0
lines changed

React/Base/RCTDisplayLink.m

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,13 @@ - (void)dealloc
9595

9696
- (void)invalidate
9797
{
98+
// ensure observer callbacks do not hold a reference to weak self via pauseCallback
99+
for (RCTModuleData *moduleData in _frameUpdateObservers) {
100+
id<RCTFrameUpdateObserver> observer = (id<RCTFrameUpdateObserver>)moduleData.instance;
101+
[observer setPauseCallback:nil];
102+
}
103+
[_frameUpdateObservers removeAllObjects]; // just to be explicit
104+
98105
[_jsDisplayLink invalidate];
99106
}
100107

0 commit comments

Comments
 (0)