Skip to content

Commit e09bb3d

Browse files
acdliteacdlite
committed
Bugfix: Fix crash when suspending in shell during useSES re-render (#27199)
This adds a regression test for a bug where, after a store mutation, the updated data causes the shell of the app to suspend. When re-rendering due to a concurrent store mutation, we must check for the RootDidNotComplete exit status again. As a follow-up, I'm going to look into to cleaning up the places where we check the exit status, so bugs like these are less likely. I think we might be able to combine most of it into a single switch statement. DiffTrain build for [201becd](201becd)
1 parent e6bc6ee commit e09bb3d

20 files changed

+1843
-2139
lines changed

‎compiled/facebook-www/REVISION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
cb3404a0ccd8b5edf5d2b90bd844742090e38f42
1+
201becd3d294b38824930a818e3412c6e04ba2eb

‎compiled/facebook-www/React-dev.classic.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (
2727
}
2828
"use strict";
2929

30-
var ReactVersion = "18.3.0-www-classic-7dfaef2c";
30+
var ReactVersion = "18.3.0-www-classic-5c10ec1e";
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,

‎compiled/facebook-www/React-dev.modern.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (
2727
}
2828
"use strict";
2929

30-
var ReactVersion = "18.3.0-www-modern-e2ad094c";
30+
var ReactVersion = "18.3.0-www-modern-2f3a20a9";
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,

‎compiled/facebook-www/React-prod.modern.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,4 +615,4 @@ exports.useSyncExternalStore = function (
615615
exports.useTransition = function () {
616616
return ReactCurrentDispatcher.current.useTransition();
617617
};
618-
exports.version = "18.3.0-www-modern-93cc7f08";
618+
exports.version = "18.3.0-www-modern-8591a478";

‎compiled/facebook-www/ReactART-dev.classic.js‎

Lines changed: 47 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-classic-36a78a59";
72+
var ReactVersion = "18.3.0-www-classic-5e6556f6";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -24246,92 +24246,71 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
2424624246
: renderRootSync(root, lanes);
2424724247

2424824248
if (exitStatus !== RootInProgress) {
24249-
if (exitStatus === RootErrored) {
24250-
// If something threw an error, try rendering one more time. We'll
24251-
// render synchronously to block concurrent data mutations, and we'll
24252-
// includes all pending updates are included. If it still fails after
24253-
// the second attempt, we'll give up and commit the resulting tree.
24254-
var originallyAttemptedLanes = lanes;
24255-
var errorRetryLanes = getLanesToRetrySynchronouslyOnError(
24256-
root,
24257-
originallyAttemptedLanes
24258-
);
24249+
var renderWasConcurrent = shouldTimeSlice;
2425924250

24260-
if (errorRetryLanes !== NoLanes) {
24261-
lanes = errorRetryLanes;
24262-
exitStatus = recoverFromConcurrentError(
24263-
root,
24264-
originallyAttemptedLanes,
24265-
errorRetryLanes
24266-
);
24267-
}
24268-
}
24251+
do {
24252+
if (exitStatus === RootDidNotComplete) {
24253+
// The render unwound without completing the tree. This happens in special
24254+
// cases where need to exit the current render without producing a
24255+
// consistent tree or committing.
24256+
markRootSuspended(root, lanes);
24257+
} else {
24258+
// The render completed.
24259+
// Check if this render may have yielded to a concurrent event, and if so,
24260+
// confirm that any newly rendered stores are consistent.
24261+
// TODO: It's possible that even a concurrent render may never have yielded
24262+
// to the main thread, if it was fast enough, or if it expired. We could
24263+
// skip the consistency check in that case, too.
24264+
var finishedWork = root.current.alternate;
2426924265

24270-
if (exitStatus === RootFatalErrored) {
24271-
var fatalError = workInProgressRootFatalError;
24272-
prepareFreshStack(root, NoLanes);
24273-
markRootSuspended(root, lanes);
24274-
ensureRootIsScheduled(root);
24275-
throw fatalError;
24276-
}
24266+
if (
24267+
renderWasConcurrent &&
24268+
!isRenderConsistentWithExternalStores(finishedWork)
24269+
) {
24270+
// A store was mutated in an interleaved event. Render again,
24271+
// synchronously, to block further mutations.
24272+
exitStatus = renderRootSync(root, lanes); // We assume the tree is now consistent because we didn't yield to any
24273+
// concurrent events.
2427724274

24278-
if (exitStatus === RootDidNotComplete) {
24279-
// The render unwound without completing the tree. This happens in special
24280-
// cases where need to exit the current render without producing a
24281-
// consistent tree or committing.
24282-
markRootSuspended(root, lanes);
24283-
} else {
24284-
// The render completed.
24285-
// Check if this render may have yielded to a concurrent event, and if so,
24286-
// confirm that any newly rendered stores are consistent.
24287-
// TODO: It's possible that even a concurrent render may never have yielded
24288-
// to the main thread, if it was fast enough, or if it expired. We could
24289-
// skip the consistency check in that case, too.
24290-
var renderWasConcurrent = !includesBlockingLane(root, lanes);
24291-
var finishedWork = root.current.alternate;
24275+
renderWasConcurrent = false; // Need to check the exit status again.
2429224276

24293-
if (
24294-
renderWasConcurrent &&
24295-
!isRenderConsistentWithExternalStores(finishedWork)
24296-
) {
24297-
// A store was mutated in an interleaved event. Render again,
24298-
// synchronously, to block further mutations.
24299-
exitStatus = renderRootSync(root, lanes); // We need to check again if something threw
24277+
continue;
24278+
} // Check if something threw
2430024279

2430124280
if (exitStatus === RootErrored) {
24302-
var _originallyAttemptedLanes = lanes;
24303-
24304-
var _errorRetryLanes = getLanesToRetrySynchronouslyOnError(
24281+
var originallyAttemptedLanes = lanes;
24282+
var errorRetryLanes = getLanesToRetrySynchronouslyOnError(
2430524283
root,
24306-
_originallyAttemptedLanes
24284+
originallyAttemptedLanes
2430724285
);
2430824286

24309-
if (_errorRetryLanes !== NoLanes) {
24310-
lanes = _errorRetryLanes;
24287+
if (errorRetryLanes !== NoLanes) {
24288+
lanes = errorRetryLanes;
2431124289
exitStatus = recoverFromConcurrentError(
2431224290
root,
24313-
_originallyAttemptedLanes,
24314-
_errorRetryLanes
24315-
); // We assume the tree is now consistent because we didn't yield to any
24316-
// concurrent events.
24291+
originallyAttemptedLanes,
24292+
errorRetryLanes
24293+
);
24294+
renderWasConcurrent = false;
2431724295
}
2431824296
}
2431924297

2432024298
if (exitStatus === RootFatalErrored) {
24321-
var _fatalError = workInProgressRootFatalError;
24299+
var fatalError = workInProgressRootFatalError;
2432224300
prepareFreshStack(root, NoLanes);
2432324301
markRootSuspended(root, lanes);
2432424302
ensureRootIsScheduled(root);
24325-
throw _fatalError;
24326-
} // FIXME: Need to check for RootDidNotComplete again. The factoring here
24327-
// isn't ideal.
24328-
} // We now have a consistent tree. The next step is either to commit it,
24329-
// or, if something suspended, wait to commit it after a timeout.
24303+
throw fatalError;
24304+
} // We now have a consistent tree. The next step is either to commit it,
24305+
// or, if something suspended, wait to commit it after a timeout.
2433024306

24331-
root.finishedWork = finishedWork;
24332-
root.finishedLanes = lanes;
24333-
finishConcurrentRender(root, exitStatus, finishedWork, lanes);
24334-
}
24307+
root.finishedWork = finishedWork;
24308+
root.finishedLanes = lanes;
24309+
finishConcurrentRender(root, exitStatus, finishedWork, lanes);
24310+
}
24311+
24312+
break;
24313+
} while (true);
2433524314
}
2433624315

2433724316
ensureRootIsScheduled(root);

‎compiled/facebook-www/ReactART-dev.modern.js‎

Lines changed: 47 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-modern-3ce11729";
72+
var ReactVersion = "18.3.0-www-modern-806e9de8";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -23911,92 +23911,71 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
2391123911
: renderRootSync(root, lanes);
2391223912

2391323913
if (exitStatus !== RootInProgress) {
23914-
if (exitStatus === RootErrored) {
23915-
// If something threw an error, try rendering one more time. We'll
23916-
// render synchronously to block concurrent data mutations, and we'll
23917-
// includes all pending updates are included. If it still fails after
23918-
// the second attempt, we'll give up and commit the resulting tree.
23919-
var originallyAttemptedLanes = lanes;
23920-
var errorRetryLanes = getLanesToRetrySynchronouslyOnError(
23921-
root,
23922-
originallyAttemptedLanes
23923-
);
23914+
var renderWasConcurrent = shouldTimeSlice;
2392423915

23925-
if (errorRetryLanes !== NoLanes) {
23926-
lanes = errorRetryLanes;
23927-
exitStatus = recoverFromConcurrentError(
23928-
root,
23929-
originallyAttemptedLanes,
23930-
errorRetryLanes
23931-
);
23932-
}
23933-
}
23916+
do {
23917+
if (exitStatus === RootDidNotComplete) {
23918+
// The render unwound without completing the tree. This happens in special
23919+
// cases where need to exit the current render without producing a
23920+
// consistent tree or committing.
23921+
markRootSuspended(root, lanes);
23922+
} else {
23923+
// The render completed.
23924+
// Check if this render may have yielded to a concurrent event, and if so,
23925+
// confirm that any newly rendered stores are consistent.
23926+
// TODO: It's possible that even a concurrent render may never have yielded
23927+
// to the main thread, if it was fast enough, or if it expired. We could
23928+
// skip the consistency check in that case, too.
23929+
var finishedWork = root.current.alternate;
2393423930

23935-
if (exitStatus === RootFatalErrored) {
23936-
var fatalError = workInProgressRootFatalError;
23937-
prepareFreshStack(root, NoLanes);
23938-
markRootSuspended(root, lanes);
23939-
ensureRootIsScheduled(root);
23940-
throw fatalError;
23941-
}
23931+
if (
23932+
renderWasConcurrent &&
23933+
!isRenderConsistentWithExternalStores(finishedWork)
23934+
) {
23935+
// A store was mutated in an interleaved event. Render again,
23936+
// synchronously, to block further mutations.
23937+
exitStatus = renderRootSync(root, lanes); // We assume the tree is now consistent because we didn't yield to any
23938+
// concurrent events.
2394223939

23943-
if (exitStatus === RootDidNotComplete) {
23944-
// The render unwound without completing the tree. This happens in special
23945-
// cases where need to exit the current render without producing a
23946-
// consistent tree or committing.
23947-
markRootSuspended(root, lanes);
23948-
} else {
23949-
// The render completed.
23950-
// Check if this render may have yielded to a concurrent event, and if so,
23951-
// confirm that any newly rendered stores are consistent.
23952-
// TODO: It's possible that even a concurrent render may never have yielded
23953-
// to the main thread, if it was fast enough, or if it expired. We could
23954-
// skip the consistency check in that case, too.
23955-
var renderWasConcurrent = !includesBlockingLane(root, lanes);
23956-
var finishedWork = root.current.alternate;
23940+
renderWasConcurrent = false; // Need to check the exit status again.
2395723941

23958-
if (
23959-
renderWasConcurrent &&
23960-
!isRenderConsistentWithExternalStores(finishedWork)
23961-
) {
23962-
// A store was mutated in an interleaved event. Render again,
23963-
// synchronously, to block further mutations.
23964-
exitStatus = renderRootSync(root, lanes); // We need to check again if something threw
23942+
continue;
23943+
} // Check if something threw
2396523944

2396623945
if (exitStatus === RootErrored) {
23967-
var _originallyAttemptedLanes = lanes;
23968-
23969-
var _errorRetryLanes = getLanesToRetrySynchronouslyOnError(
23946+
var originallyAttemptedLanes = lanes;
23947+
var errorRetryLanes = getLanesToRetrySynchronouslyOnError(
2397023948
root,
23971-
_originallyAttemptedLanes
23949+
originallyAttemptedLanes
2397223950
);
2397323951

23974-
if (_errorRetryLanes !== NoLanes) {
23975-
lanes = _errorRetryLanes;
23952+
if (errorRetryLanes !== NoLanes) {
23953+
lanes = errorRetryLanes;
2397623954
exitStatus = recoverFromConcurrentError(
2397723955
root,
23978-
_originallyAttemptedLanes,
23979-
_errorRetryLanes
23980-
); // We assume the tree is now consistent because we didn't yield to any
23981-
// concurrent events.
23956+
originallyAttemptedLanes,
23957+
errorRetryLanes
23958+
);
23959+
renderWasConcurrent = false;
2398223960
}
2398323961
}
2398423962

2398523963
if (exitStatus === RootFatalErrored) {
23986-
var _fatalError = workInProgressRootFatalError;
23964+
var fatalError = workInProgressRootFatalError;
2398723965
prepareFreshStack(root, NoLanes);
2398823966
markRootSuspended(root, lanes);
2398923967
ensureRootIsScheduled(root);
23990-
throw _fatalError;
23991-
} // FIXME: Need to check for RootDidNotComplete again. The factoring here
23992-
// isn't ideal.
23993-
} // We now have a consistent tree. The next step is either to commit it,
23994-
// or, if something suspended, wait to commit it after a timeout.
23968+
throw fatalError;
23969+
} // We now have a consistent tree. The next step is either to commit it,
23970+
// or, if something suspended, wait to commit it after a timeout.
2399523971

23996-
root.finishedWork = finishedWork;
23997-
root.finishedLanes = lanes;
23998-
finishConcurrentRender(root, exitStatus, finishedWork, lanes);
23999-
}
23972+
root.finishedWork = finishedWork;
23973+
root.finishedLanes = lanes;
23974+
finishConcurrentRender(root, exitStatus, finishedWork, lanes);
23975+
}
23976+
23977+
break;
23978+
} while (true);
2400023979
}
2400123980

2400223981
ensureRootIsScheduled(root);

0 commit comments

Comments
 (0)