Skip to content

Commit

Permalink
Fabric: Explicit Scheduler creation and destruction management in RCT…
Browse files Browse the repository at this point in the history
…SurfacePresenter

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
  • Loading branch information
shergin authored and facebook-github-bot committed Sep 11, 2019
1 parent 46d6e2a commit 83f8d13
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,8 @@ - (void)setFrame:(CGRect)frame
&maximumSize
);

if (RCTSurfaceStageIsRunning(_stage)) {
[_surface setMinimumSize:minimumSize
maximumSize:maximumSize];
}
}

- (CGSize)intrinsicContentSize
Expand Down
65 changes: 29 additions & 36 deletions React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ @interface RCTSurfacePresenter () <RCTSchedulerDelegate, RCTMountingManagerDeleg

@implementation RCTSurfacePresenter {
std::mutex _schedulerMutex;
std::mutex _contextContainerMutex;
RCTScheduler
*_Nullable _scheduler; // Thread-safe. Mutation of the instance variable is protected by `_schedulerMutex`.
RCTMountingManager *_mountingManager; // Thread-safe.
RCTSurfaceRegistry *_surfaceRegistry; // Thread-safe.
RCTBridge *_bridge; // Unsafe. We are moving away from Bridge.
RCTBridge *_batchedBridge;
std::shared_ptr<const ReactNativeConfig> _reactNativeConfig;
ContextContainer::Shared _contextContainer;
better::shared_mutex _observerListMutex;
NSMutableArray<id<RCTSurfacePresenterObserver>> *_observers;
RCTImageLoader *_imageLoader;
Expand Down Expand Up @@ -88,6 +88,8 @@ - (instancetype)initWithBridge:(RCTBridge *_Nullable)bridge
_reactNativeConfig = std::make_shared<const EmptyReactNativeConfig>();
}

_contextContainer = std::make_shared<ContextContainer>();

_observers = [NSMutableArray array];

[[NSNotificationCenter defaultCenter] addObserver:self
Expand All @@ -98,6 +100,8 @@ - (instancetype)initWithBridge:(RCTBridge *_Nullable)bridge
selector:@selector(handleJavaScriptDidLoadNotification:)
name:RCTJavaScriptDidLoadNotification
object:_bridge];

[self _createScheduler];
}

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

#pragma mark - Private

- (RCTScheduler *)_scheduler
- (nullable RCTScheduler *)_scheduler
{
std::lock_guard<std::mutex> lock(_schedulerMutex);
return _scheduler;
}

if (_scheduler) {
return _scheduler;
}
- (void)_createScheduler
{
std::lock_guard<std::mutex> lock(_schedulerMutex);

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

auto runtimeExecutor = [self getRuntimeExecutor];

[self _updateContextContainerIfNeeded_DEPRECATED];

auto toolbox = SchedulerToolbox{};
toolbox.contextContainer = self.contextContainer;
toolbox.componentRegistryFactory = componentRegistryFactory;
Expand All @@ -218,11 +226,13 @@ - (RCTScheduler *)_scheduler

_scheduler = [[RCTScheduler alloc] initWithToolbox:toolbox];
_scheduler.delegate = self;

return _scheduler;
}

@synthesize contextContainer = _contextContainer;
- (void)_destroyScheduler
{
std::lock_guard<std::mutex> lock(_schedulerMutex);
_scheduler = nil;
}

- (RuntimeExecutor)getRuntimeExecutor
{
Expand Down Expand Up @@ -250,16 +260,6 @@ - (RuntimeExecutor)getRuntimeExecutor

- (ContextContainer::Shared)contextContainer
{
std::lock_guard<std::mutex> lock(_contextContainerMutex);

if (_contextContainer) {
return _contextContainer;
}

_contextContainer = std::make_shared<ContextContainer>();

[self _updateContextContainerIfNeeded_DEPRECATED];

return _contextContainer;
}

Expand Down Expand Up @@ -409,34 +409,27 @@ - (void)mountingManager:(RCTMountingManager *)mountingManager didMountComponents

- (void)handleBridgeWillReloadNotification:(NSNotification *)notification
{
{
std::lock_guard<std::mutex> lock(_schedulerMutex);
if (!_scheduler) {
// Seems we are already in the realoding process.
return;
}
if (!self._scheduler) {
// Seems we are already in the reloading process.
return;
}

[self _stopAllSurfaces];

{
std::lock_guard<std::mutex> lock(_schedulerMutex);
_scheduler = nil;
}
[self _destroyScheduler];
}

- (void)handleJavaScriptDidLoadNotification:(NSNotification *)notification
{
RCTBridge *bridge = notification.userInfo[@"bridge"];
if (bridge != _batchedBridge) {
_batchedBridge = bridge;
if (bridge == _batchedBridge) {
// Nothing really changed.
return;
}

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

[self _startAllSurfaces];
}
[self _createScheduler];
[self _startAllSurfaces];
}

@end

0 comments on commit 83f8d13

Please sign in to comment.