Skip to content

Commit

Permalink
Bugfix: Synchronous ping during render phase sometimes unwinds the st…
Browse files Browse the repository at this point in the history
…ack, leading to crash (#25851)

I found this bug when working on a different task.

`pingSuspendedRoot` sometimes calls `prepareFreshStack` to interupt the
work-in-progress tree and force a restart from the root. The idea is
that if the current render is already in a state where it be blocked
from committing, and there's new data that could unblock it, we might as
well restart from the beginning.

The problem is that this is only safe to do if `pingSuspendedRoot` is
called from a non-React task, like an event handler or a microtask.
While this is usually the case, it's entirely possible for a thenable to
resolve (i.e. to call `pingSuspendedRoot`) synchronously while the
render phase is already executing. If that happens, and work loop
attempts to unwind the stack, it causes the render phase to crash.

DiffTrain build for [5fcf1a4](5fcf1a4)
[View git log for this commit](https://github.com/facebook/react/commits/5fcf1a4b4c2150a1b9fe0de0144a82a053c63966)
  • Loading branch information
acdlite committed Dec 22, 2022
1 parent ecbf597 commit 769cfc7
Show file tree
Hide file tree
Showing 28 changed files with 90 additions and 66 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2b1fb91a55deb9b7b60452cb57184c2f182a42fd
5fcf1a4b4c2150a1b9fe0de0144a82a053c63966
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION_TRANSFORMS
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2b1fb91a55deb9b7b60452cb57184c2f182a42fd
5fcf1a4b4c2150a1b9fe0de0144a82a053c63966
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-classic-5fcf1a4b4-20221221";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-modern-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-modern-5fcf1a4b4-20221221";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -643,4 +643,4 @@ exports.useSyncExternalStore = function(
);
};
exports.useTransition = useTransition;
exports.version = "18.3.0-www-classic-2b1fb91a5-20221220";
exports.version = "18.3.0-www-classic-5fcf1a4b4-20221221";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -635,4 +635,4 @@ exports.useSyncExternalStore = function(
);
};
exports.useTransition = useTransition;
exports.version = "18.3.0-www-modern-2b1fb91a5-20221220";
exports.version = "18.3.0-www-modern-5fcf1a4b4-20221221";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ exports.useSyncExternalStore = function(
);
};
exports.useTransition = useTransition;
exports.version = "18.3.0-www-classic-2b1fb91a5-20221220";
exports.version = "18.3.0-www-classic-5fcf1a4b4-20221221";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ exports.useSyncExternalStore = function(
);
};
exports.useTransition = useTransition;
exports.version = "18.3.0-www-modern-2b1fb91a5-20221220";
exports.version = "18.3.0-www-modern-5fcf1a4b4-20221221";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
9 changes: 6 additions & 3 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-classic-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-classic-5fcf1a4b4-20221221";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -25766,8 +25766,11 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
includesOnlyRetries(workInProgressRootRenderLanes) &&
now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS)
) {
// Restart from the root.
prepareFreshStack(root, NoLanes);
// Force a restart from the root by unwinding the stack. Unless this is
// being called from the render phase, because that would cause a crash.
if ((executionContext & RenderContext) === NoContext) {
prepareFreshStack(root, NoLanes);
}
} else {
// Even though we can't restart right now, we might get an
// opportunity later. So we mark this render as having a ping.
Expand Down
9 changes: 6 additions & 3 deletions compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-modern-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-modern-5fcf1a4b4-20221221";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -25455,8 +25455,11 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
includesOnlyRetries(workInProgressRootRenderLanes) &&
now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS)
) {
// Restart from the root.
prepareFreshStack(root, NoLanes);
// Force a restart from the root by unwinding the stack. Unless this is
// being called from the render phase, because that would cause a crash.
if ((executionContext & RenderContext) === NoContext) {
prepareFreshStack(root, NoLanes);
}
} else {
// Even though we can't restart right now, we might get an
// opportunity later. So we mark this render as having a ping.
Expand Down
6 changes: 3 additions & 3 deletions compiled/facebook-www/ReactART-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -8862,7 +8862,7 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
(workInProgressRootRenderLanes & 125829120) ===
workInProgressRootRenderLanes &&
500 > now() - globalMostRecentFallbackTime)
? prepareFreshStack(root, 0)
? 0 === (executionContext & 2) && prepareFreshStack(root, 0)
: (workInProgressRootPingedLanes |= pingedLanes));
ensureRootIsScheduled(root, wakeable);
}
Expand Down Expand Up @@ -9827,7 +9827,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-classic-2b1fb91a5-20221220",
version: "18.3.0-www-classic-5fcf1a4b4-20221221",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1338 = {
Expand Down Expand Up @@ -9858,7 +9858,7 @@ var internals$jscomp$inline_1338 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-next-2b1fb91a5-20221220"
reconcilerVersion: "18.3.0-next-5fcf1a4b4-20221221"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1339 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
6 changes: 3 additions & 3 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -8596,7 +8596,7 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
(workInProgressRootRenderLanes & 125829120) ===
workInProgressRootRenderLanes &&
500 > now() - globalMostRecentFallbackTime)
? prepareFreshStack(root, 0)
? 0 === (executionContext & 2) && prepareFreshStack(root, 0)
: (workInProgressRootPingedLanes |= pingedLanes));
ensureRootIsScheduled(root, wakeable);
}
Expand Down Expand Up @@ -9494,7 +9494,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-2b1fb91a5-20221220",
version: "18.3.0-www-modern-5fcf1a4b4-20221221",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1329 = {
Expand Down Expand Up @@ -9525,7 +9525,7 @@ var internals$jscomp$inline_1329 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-next-2b1fb91a5-20221220"
reconcilerVersion: "18.3.0-next-5fcf1a4b4-20221221"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1330 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
9 changes: 6 additions & 3 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -40807,8 +40807,11 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
includesOnlyRetries(workInProgressRootRenderLanes) &&
now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS)
) {
// Restart from the root.
prepareFreshStack(root, NoLanes);
// Force a restart from the root by unwinding the stack. Unless this is
// being called from the render phase, because that would cause a crash.
if ((executionContext & RenderContext) === NoContext) {
prepareFreshStack(root, NoLanes);
}
} else {
// Even though we can't restart right now, we might get an
// opportunity later. So we mark this render as having a ping.
Expand Down Expand Up @@ -42643,7 +42646,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-classic-5fcf1a4b4-20221221";

function createPortal(
children,
Expand Down
9 changes: 6 additions & 3 deletions compiled/facebook-www/ReactDOM-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -40531,8 +40531,11 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
includesOnlyRetries(workInProgressRootRenderLanes) &&
now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS)
) {
// Restart from the root.
prepareFreshStack(root, NoLanes);
// Force a restart from the root by unwinding the stack. Unless this is
// being called from the render phase, because that would cause a crash.
if ((executionContext & RenderContext) === NoContext) {
prepareFreshStack(root, NoLanes);
}
} else {
// Even though we can't restart right now, we might get an
// opportunity later. So we mark this render as having a ping.
Expand Down Expand Up @@ -42367,7 +42370,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-modern-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-modern-5fcf1a4b4-20221221";

function createPortal(
children,
Expand Down
8 changes: 4 additions & 4 deletions compiled/facebook-www/ReactDOM-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -14222,7 +14222,7 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
(workInProgressRootRenderLanes & 125829120) ===
workInProgressRootRenderLanes &&
500 > now() - globalMostRecentFallbackTime)
? prepareFreshStack(root, 0)
? 0 === (executionContext & 2) && prepareFreshStack(root, 0)
: (workInProgressRootPingedLanes |= pingedLanes));
ensureRootIsScheduled(root, wakeable);
}
Expand Down Expand Up @@ -15580,7 +15580,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1770 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-classic-2b1fb91a5-20221220",
version: "18.3.0-www-classic-5fcf1a4b4-20221221",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2157 = {
Expand Down Expand Up @@ -15610,7 +15610,7 @@ var internals$jscomp$inline_2157 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-next-2b1fb91a5-20221220"
reconcilerVersion: "18.3.0-next-5fcf1a4b4-20221221"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2158 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -15852,4 +15852,4 @@ exports.unstable_renderSubtreeIntoContainer = function(
);
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-next-2b1fb91a5-20221220";
exports.version = "18.3.0-next-5fcf1a4b4-20221221";
8 changes: 4 additions & 4 deletions compiled/facebook-www/ReactDOM-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -14000,7 +14000,7 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
(workInProgressRootRenderLanes & 125829120) ===
workInProgressRootRenderLanes &&
500 > now() - globalMostRecentFallbackTime)
? prepareFreshStack(root, 0)
? 0 === (executionContext & 2) && prepareFreshStack(root, 0)
: (workInProgressRootPingedLanes |= pingedLanes));
ensureRootIsScheduled(root, wakeable);
}
Expand Down Expand Up @@ -15141,7 +15141,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1738 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-modern-2b1fb91a5-20221220",
version: "18.3.0-www-modern-5fcf1a4b4-20221221",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2132 = {
Expand Down Expand Up @@ -15172,7 +15172,7 @@ var internals$jscomp$inline_2132 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-next-2b1fb91a5-20221220"
reconcilerVersion: "18.3.0-next-5fcf1a4b4-20221221"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2133 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -15360,4 +15360,4 @@ exports.unstable_flushControlled = function(fn) {
}
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-next-2b1fb91a5-20221220";
exports.version = "18.3.0-next-5fcf1a4b4-20221221";
8 changes: 4 additions & 4 deletions compiled/facebook-www/ReactDOM-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -14958,7 +14958,7 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
(workInProgressRootRenderLanes & 125829120) ===
workInProgressRootRenderLanes &&
500 > now() - globalMostRecentFallbackTime)
? prepareFreshStack(root, 0)
? 0 === (executionContext & 2) && prepareFreshStack(root, 0)
: (workInProgressRootPingedLanes |= pingedLanes));
ensureRootIsScheduled(root, wakeable);
}
Expand Down Expand Up @@ -16352,7 +16352,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1844 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-classic-2b1fb91a5-20221220",
version: "18.3.0-www-classic-5fcf1a4b4-20221221",
rendererPackageName: "react-dom"
};
(function(internals) {
Expand Down Expand Up @@ -16396,7 +16396,7 @@ var devToolsConfig$jscomp$inline_1844 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-next-2b1fb91a5-20221220"
reconcilerVersion: "18.3.0-next-5fcf1a4b4-20221221"
});
assign(Internals, {
ReactBrowserEventEmitter: {
Expand Down Expand Up @@ -16625,7 +16625,7 @@ exports.unstable_renderSubtreeIntoContainer = function(
);
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-next-2b1fb91a5-20221220";
exports.version = "18.3.0-next-5fcf1a4b4-20221221";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
8 changes: 4 additions & 4 deletions compiled/facebook-www/ReactDOM-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -14726,7 +14726,7 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
(workInProgressRootRenderLanes & 125829120) ===
workInProgressRootRenderLanes &&
500 > now() - globalMostRecentFallbackTime)
? prepareFreshStack(root, 0)
? 0 === (executionContext & 2) && prepareFreshStack(root, 0)
: (workInProgressRootPingedLanes |= pingedLanes));
ensureRootIsScheduled(root, wakeable);
}
Expand Down Expand Up @@ -15903,7 +15903,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1812 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-modern-2b1fb91a5-20221220",
version: "18.3.0-www-modern-5fcf1a4b4-20221221",
rendererPackageName: "react-dom"
};
(function(internals) {
Expand Down Expand Up @@ -15948,7 +15948,7 @@ var devToolsConfig$jscomp$inline_1812 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-next-2b1fb91a5-20221220"
reconcilerVersion: "18.3.0-next-5fcf1a4b4-20221221"
});
exports.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED = Internals;
exports.createPortal = function(children, container) {
Expand Down Expand Up @@ -16123,7 +16123,7 @@ exports.unstable_flushControlled = function(fn) {
}
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-next-2b1fb91a5-20221220";
exports.version = "18.3.0-next-5fcf1a4b4-20221221";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactDOMServer-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-classic-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-classic-5fcf1a4b4-20221221";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactDOMServer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-modern-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-modern-5fcf1a4b4-20221221";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactDOMServer-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -3633,4 +3633,4 @@ exports.renderToString = function(children, options) {
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
);
};
exports.version = "18.3.0-www-classic-2b1fb91a5-20221220";
exports.version = "18.3.0-www-classic-5fcf1a4b4-20221221";
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactDOMServer-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -3546,4 +3546,4 @@ exports.renderToString = function(children, options) {
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
);
};
exports.version = "18.3.0-www-modern-2b1fb91a5-20221220";
exports.version = "18.3.0-www-modern-5fcf1a4b4-20221221";
9 changes: 6 additions & 3 deletions compiled/facebook-www/ReactDOMTesting-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -29600,8 +29600,11 @@ function pingSuspendedRoot(root, wakeable, pingedLanes) {
includesOnlyRetries(workInProgressRootRenderLanes) &&
now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS)
) {
// Restart from the root.
prepareFreshStack(root, NoLanes);
// Force a restart from the root by unwinding the stack. Unless this is
// being called from the render phase, because that would cause a crash.
if ((executionContext & RenderContext) === NoContext) {
prepareFreshStack(root, NoLanes);
}
} else {
// Even though we can't restart right now, we might get an
// opportunity later. So we mark this render as having a ping.
Expand Down Expand Up @@ -31168,7 +31171,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-2b1fb91a5-20221220";
var ReactVersion = "18.3.0-www-classic-5fcf1a4b4-20221221";

function createPortal(
children,
Expand Down
Loading

0 comments on commit 769cfc7

Please sign in to comment.