Skip to content

Commit 83f8d13

Browse files
sherginfacebook-github-bot
authored andcommitted
Fabric: Explicit Scheduler creation and destruction management in RCTSurfacePresenter
Summary: Previously, the `_scheduler` method in `RCTSurfacePresenter` was implemented as a lazy getter. The only problem with that is that Scheduler instance might be (re)created in the middle of the hot-reloading process (e.g. external request to relayout some Surface might trigger that). Since it does not make any sense to create an empty Scheduler during the reloading process, now the Scheduler creation only happens in constructor and right after the VM is reloaded. Reviewed By: JoshuaGross Differential Revision: D17299441 fbshipit-source-id: 273451bbb03e8cdf532131adfdf3bc60c34e997e
1 parent 46d6e2a commit 83f8d13

2 files changed

Lines changed: 29 additions & 38 deletions

File tree

‎React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingView.mm‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,8 @@ - (void)setFrame:(CGRect)frame
8080
&maximumSize
8181
);
8282

83-
if (RCTSurfaceStageIsRunning(_stage)) {
8483
[_surface setMinimumSize:minimumSize
8584
maximumSize:maximumSize];
86-
}
8785
}
8886

8987
- (CGSize)intrinsicContentSize

‎React/Fabric/RCTSurfacePresenter.mm‎

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,14 @@ @interface RCTSurfacePresenter () <RCTSchedulerDelegate, RCTMountingManagerDeleg
5151

5252
@implementation RCTSurfacePresenter {
5353
std::mutex _schedulerMutex;
54-
std::mutex _contextContainerMutex;
5554
RCTScheduler
5655
*_Nullable _scheduler; // Thread-safe. Mutation of the instance variable is protected by `_schedulerMutex`.
5756
RCTMountingManager *_mountingManager; // Thread-safe.
5857
RCTSurfaceRegistry *_surfaceRegistry; // Thread-safe.
5958
RCTBridge *_bridge; // Unsafe. We are moving away from Bridge.
6059
RCTBridge *_batchedBridge;
6160
std::shared_ptr<const ReactNativeConfig> _reactNativeConfig;
61+
ContextContainer::Shared _contextContainer;
6262
better::shared_mutex _observerListMutex;
6363
NSMutableArray<id<RCTSurfacePresenterObserver>> *_observers;
6464
RCTImageLoader *_imageLoader;
@@ -88,6 +88,8 @@ - (instancetype)initWithBridge:(RCTBridge *_Nullable)bridge
8888
_reactNativeConfig = std::make_shared<const EmptyReactNativeConfig>();
8989
}
9090

91+
_contextContainer = std::make_shared<ContextContainer>();
92+
9193
_observers = [NSMutableArray array];
9294

9395
[[NSNotificationCenter defaultCenter] addObserver:self
@@ -98,6 +100,8 @@ - (instancetype)initWithBridge:(RCTBridge *_Nullable)bridge
98100
selector:@selector(handleJavaScriptDidLoadNotification:)
99101
name:RCTJavaScriptDidLoadNotification
100102
object:_bridge];
103+
104+
[self _createScheduler];
101105
}
102106

103107
return self;
@@ -186,13 +190,15 @@ - (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictiona
186190

187191
#pragma mark - Private
188192

189-
- (RCTScheduler *)_scheduler
193+
- (nullable RCTScheduler *)_scheduler
190194
{
191195
std::lock_guard<std::mutex> lock(_schedulerMutex);
196+
return _scheduler;
197+
}
192198

193-
if (_scheduler) {
194-
return _scheduler;
195-
}
199+
- (void)_createScheduler
200+
{
201+
std::lock_guard<std::mutex> lock(_schedulerMutex);
196202

197203
auto componentRegistryFactory = [factory = wrapManagedObject(self.componentViewFactory)](
198204
EventDispatcher::Weak const &eventDispatcher,
@@ -203,6 +209,8 @@ - (RCTScheduler *)_scheduler
203209

204210
auto runtimeExecutor = [self getRuntimeExecutor];
205211

212+
[self _updateContextContainerIfNeeded_DEPRECATED];
213+
206214
auto toolbox = SchedulerToolbox{};
207215
toolbox.contextContainer = self.contextContainer;
208216
toolbox.componentRegistryFactory = componentRegistryFactory;
@@ -218,11 +226,13 @@ - (RCTScheduler *)_scheduler
218226

219227
_scheduler = [[RCTScheduler alloc] initWithToolbox:toolbox];
220228
_scheduler.delegate = self;
221-
222-
return _scheduler;
223229
}
224230

225-
@synthesize contextContainer = _contextContainer;
231+
- (void)_destroyScheduler
232+
{
233+
std::lock_guard<std::mutex> lock(_schedulerMutex);
234+
_scheduler = nil;
235+
}
226236

227237
- (RuntimeExecutor)getRuntimeExecutor
228238
{
@@ -250,16 +260,6 @@ - (RuntimeExecutor)getRuntimeExecutor
250260

251261
- (ContextContainer::Shared)contextContainer
252262
{
253-
std::lock_guard<std::mutex> lock(_contextContainerMutex);
254-
255-
if (_contextContainer) {
256-
return _contextContainer;
257-
}
258-
259-
_contextContainer = std::make_shared<ContextContainer>();
260-
261-
[self _updateContextContainerIfNeeded_DEPRECATED];
262-
263263
return _contextContainer;
264264
}
265265

@@ -409,34 +409,27 @@ - (void)mountingManager:(RCTMountingManager *)mountingManager didMountComponents
409409

410410
- (void)handleBridgeWillReloadNotification:(NSNotification *)notification
411411
{
412-
{
413-
std::lock_guard<std::mutex> lock(_schedulerMutex);
414-
if (!_scheduler) {
415-
// Seems we are already in the realoding process.
416-
return;
417-
}
412+
if (!self._scheduler) {
413+
// Seems we are already in the reloading process.
414+
return;
418415
}
419416

420417
[self _stopAllSurfaces];
421-
422-
{
423-
std::lock_guard<std::mutex> lock(_schedulerMutex);
424-
_scheduler = nil;
425-
}
418+
[self _destroyScheduler];
426419
}
427420

428421
- (void)handleJavaScriptDidLoadNotification:(NSNotification *)notification
429422
{
430423
RCTBridge *bridge = notification.userInfo[@"bridge"];
431-
if (bridge != _batchedBridge) {
432-
_batchedBridge = bridge;
424+
if (bridge == _batchedBridge) {
425+
// Nothing really changed.
426+
return;
427+
}
433428

434-
// Some of the injected dependencies are tight to a particular instance of Bridge,
435-
// so they need to be reinjected.
436-
[self _updateContextContainerIfNeeded_DEPRECATED];
429+
_batchedBridge = bridge;
437430

438-
[self _startAllSurfaces];
439-
}
431+
[self _createScheduler];
432+
[self _startAllSurfaces];
440433
}
441434

442435
@end

0 commit comments

Comments
 (0)