From 5f0435fb852cbb8abfe6cab77784ebddf1ad38ac Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 18 Nov 2019 14:22:55 -0800 Subject: [PATCH] Fabric: Support for interleaving/followup transactions in RCTMountingManager (iOS) Summary: Imagine a case where we initiate a synchronous state update right in the middle of the mount transaction. With the current implementation, the mount transaction caused by the change will be mounted right inside the in-flight transaction, which will probably cause a crash or incorrect mounting side-effects (which will cause a crash later). Instead of executing all mounting instructions cased by the state change, we actually need to execute them right after the end of the current transaction (synchronously, inside the same main run loop tick). This diff implements exactly this. Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D18444730 fbshipit-source-id: 3e777a7aa70ff28205d40588970c7478869b6899 --- React/Fabric/Mounting/RCTMountingManager.mm | 30 +++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/React/Fabric/Mounting/RCTMountingManager.mm b/React/Fabric/Mounting/RCTMountingManager.mm index 709b2e302cb260..e0096438ff16cf 100644 --- a/React/Fabric/Mounting/RCTMountingManager.mm +++ b/React/Fabric/Mounting/RCTMountingManager.mm @@ -207,6 +207,8 @@ static void RNPerformMountInstructions( @implementation RCTMountingManager { RCTMountingTransactionObserverCoordinator _observerCoordinator; + BOOL _transactionInFlight; + BOOL _followUpTransactionRequired; } - (instancetype)init @@ -225,14 +227,14 @@ - (void)scheduleTransaction:(MountingCoordinator::Shared const &)mountingCoordin // * No need to do a thread jump; // * No need to do expensive copy of all mutations; // * No need to allocate a block. - [self mountMutations:mountingCoordinator]; + [self initiateTransaction:mountingCoordinator]; return; } auto mountingCoordinatorCopy = mountingCoordinator; RCTExecuteOnMainQueue(^{ RCTAssertMainQueue(); - [self mountMutations:mountingCoordinatorCopy]; + [self initiateTransaction:mountingCoordinatorCopy]; }); } @@ -252,9 +254,28 @@ - (void)dispatchCommand:(ReactTag)reactTag commandName:(NSString *)commandName a }); } -- (void)mountMutations:(MountingCoordinator::Shared const &)mountingCoordinator +- (void)initiateTransaction:(MountingCoordinator::Shared const &)mountingCoordinator { - SystraceSection s("-[RCTMountingManager mountMutations:]"); + SystraceSection s("-[RCTMountingManager initiateTransaction:]"); + RCTAssertMainQueue(); + + if (_transactionInFlight) { + _followUpTransactionRequired = YES; + return; + } + + do { + _followUpTransactionRequired = NO; + _transactionInFlight = YES; + [self performTransaction:mountingCoordinator]; + _transactionInFlight = NO; + } while (_followUpTransactionRequired); +} + +- (void)performTransaction:(MountingCoordinator::Shared const &)mountingCoordinator +{ + SystraceSection s("-[RCTMountingManager performTransaction:]"); + RCTAssertMainQueue(); auto transaction = mountingCoordinator->pullTransaction(); if (!transaction.has_value()) { @@ -268,7 +289,6 @@ - (void)mountMutations:(MountingCoordinator::Shared const &)mountingCoordinator return; } - RCTAssertMainQueue(); auto telemetry = transaction->getTelemetry(); auto number = transaction->getNumber();