Skip to content

Commit

Permalink
[Fizz][Float] Do not write after closing the stream (#27541)
Browse files Browse the repository at this point in the history
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.

fixes: #27540

DiffTrain build for [601e5c3](601e5c3)
  • Loading branch information
gnoff committed Oct 18, 2023
1 parent 1050447 commit 24721ff
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 67 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
75c1bd7ee7e4a87a4dd0f560e157c57957ef823b
601e5c38505ebc0ee099d8666b2f7a8b03159ac4
2 changes: 1 addition & 1 deletion 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-f972e958";
var ReactVersion = "18.3.0-www-modern-6d0bc8c7";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down
4 changes: 2 additions & 2 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -9848,7 +9848,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-4a0e1ba5",
version: "18.3.0-www-modern-75350665",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1302 = {
Expand Down Expand Up @@ -9879,7 +9879,7 @@ var internals$jscomp$inline_1302 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-4a0e1ba5"
reconcilerVersion: "18.3.0-www-modern-75350665"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1303 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
21 changes: 17 additions & 4 deletions 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-c2fb22b9";
var ReactVersion = "18.3.0-www-classic-89c1183f";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -13209,7 +13209,10 @@ function flushCompletedQueues(request, destination) {
}
} // We're done.

close(destination);
close(destination); // We need to stop flowing now because we do not want any async contexts which might call
// float methods to initiate any flushes after this point

stopFlowing(request);
}
}
}
Expand All @@ -13231,10 +13234,17 @@ function enqueueFlush(request) {
// happen when we start flowing again
request.destination !== null
) {
var destination = request.destination;
request.flushScheduled = true;
scheduleWork(function () {
return flushCompletedQueues(request, destination);
// We need to existence check destination again here because it might go away
// in between the enqueueFlush call and the work execution
var destination = request.destination;

if (destination) {
flushCompletedQueues(request, destination);
} else {
request.flushScheduled = false;
}
});
}
}
Expand Down Expand Up @@ -13264,6 +13274,9 @@ function startFlowing(request, destination) {
fatalError(request, error);
}
}
function stopFlowing(request) {
request.destination = null;
} // This is called to early terminate a request. It puts all pending boundaries in client rendered state.

function abort(request, reason) {
try {
Expand Down
21 changes: 17 additions & 4 deletions 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-f972e958";
var ReactVersion = "18.3.0-www-modern-6d0bc8c7";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -12957,7 +12957,10 @@ function flushCompletedQueues(request, destination) {
}
} // We're done.

close(destination);
close(destination); // We need to stop flowing now because we do not want any async contexts which might call
// float methods to initiate any flushes after this point

stopFlowing(request);
}
}
}
Expand All @@ -12979,10 +12982,17 @@ function enqueueFlush(request) {
// happen when we start flowing again
request.destination !== null
) {
var destination = request.destination;
request.flushScheduled = true;
scheduleWork(function () {
return flushCompletedQueues(request, destination);
// We need to existence check destination again here because it might go away
// in between the enqueueFlush call and the work execution
var destination = request.destination;

if (destination) {
flushCompletedQueues(request, destination);
} else {
request.flushScheduled = false;
}
});
}
}
Expand Down Expand Up @@ -13012,6 +13022,9 @@ function startFlowing(request, destination) {
fatalError(request, error);
}
}
function stopFlowing(request) {
request.destination = null;
} // This is called to early terminate a request. It puts all pending boundaries in client rendered state.

function abort(request, reason) {
try {
Expand Down
53 changes: 28 additions & 25 deletions compiled/facebook-www/ReactDOMServer-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -2287,24 +2287,24 @@ function hoistStylesheetDependency(stylesheet) {
function createRenderState(resumableState, generateStaticMarkup) {
var idPrefix = resumableState.idPrefix;
resumableState = idPrefix + "P:";
var JSCompiler_object_inline_segmentPrefix_1570 = idPrefix + "S:";
var JSCompiler_object_inline_segmentPrefix_1574 = idPrefix + "S:";
idPrefix += "B:";
var JSCompiler_object_inline_preconnects_1582 = new Set(),
JSCompiler_object_inline_fontPreloads_1583 = new Set(),
JSCompiler_object_inline_highImagePreloads_1584 = new Set(),
JSCompiler_object_inline_styles_1585 = new Map(),
JSCompiler_object_inline_bootstrapScripts_1586 = new Set(),
JSCompiler_object_inline_scripts_1587 = new Set(),
JSCompiler_object_inline_bulkPreloads_1588 = new Set(),
JSCompiler_object_inline_preloads_1589 = {
var JSCompiler_object_inline_preconnects_1586 = new Set(),
JSCompiler_object_inline_fontPreloads_1587 = new Set(),
JSCompiler_object_inline_highImagePreloads_1588 = new Set(),
JSCompiler_object_inline_styles_1589 = new Map(),
JSCompiler_object_inline_bootstrapScripts_1590 = new Set(),
JSCompiler_object_inline_scripts_1591 = new Set(),
JSCompiler_object_inline_bulkPreloads_1592 = new Set(),
JSCompiler_object_inline_preloads_1593 = {
images: new Map(),
stylesheets: new Map(),
scripts: new Map(),
moduleScripts: new Map()
};
return {
placeholderPrefix: resumableState,
segmentPrefix: JSCompiler_object_inline_segmentPrefix_1570,
segmentPrefix: JSCompiler_object_inline_segmentPrefix_1574,
boundaryPrefix: idPrefix,
startInlineScript: "<script>",
htmlChunks: null,
Expand All @@ -2316,14 +2316,14 @@ function createRenderState(resumableState, generateStaticMarkup) {
importMapChunks: [],
preloadChunks: [],
hoistableChunks: [],
preconnects: JSCompiler_object_inline_preconnects_1582,
fontPreloads: JSCompiler_object_inline_fontPreloads_1583,
highImagePreloads: JSCompiler_object_inline_highImagePreloads_1584,
styles: JSCompiler_object_inline_styles_1585,
bootstrapScripts: JSCompiler_object_inline_bootstrapScripts_1586,
scripts: JSCompiler_object_inline_scripts_1587,
bulkPreloads: JSCompiler_object_inline_bulkPreloads_1588,
preloads: JSCompiler_object_inline_preloads_1589,
preconnects: JSCompiler_object_inline_preconnects_1586,
fontPreloads: JSCompiler_object_inline_fontPreloads_1587,
highImagePreloads: JSCompiler_object_inline_highImagePreloads_1588,
styles: JSCompiler_object_inline_styles_1589,
bootstrapScripts: JSCompiler_object_inline_bootstrapScripts_1590,
scripts: JSCompiler_object_inline_scripts_1591,
bulkPreloads: JSCompiler_object_inline_bulkPreloads_1592,
preloads: JSCompiler_object_inline_preloads_1593,
boundaryResources: null,
stylesToHoist: !1,
generateStaticMarkup: generateStaticMarkup
Expand Down Expand Up @@ -4930,16 +4930,17 @@ function flushCompletedQueues(request, destination) {
0 === request.clientRenderedBoundaries.length &&
0 === request.completedBoundaries.length &&
((request.flushScheduled = !1),
(request = request.resumableState),
request.hasBody &&
(i = request.resumableState),
i.hasBody &&
(destination.push("</"),
destination.push("body"),
destination.push(">")),
request.hasHtml &&
i.hasHtml &&
(destination.push("</"),
destination.push("html"),
destination.push(">")),
destination.push(null));
destination.push(null),
(request.destination = null));
}
}
function enqueueFlush(request) {
Expand All @@ -4948,9 +4949,11 @@ function enqueueFlush(request) {
0 === request.pingedTasks.length &&
null !== request.destination
) {
var destination = request.destination;
request.flushScheduled = !0;
flushCompletedQueues(request, destination);
var destination = request.destination;
destination
? flushCompletedQueues(request, destination)
: (request.flushScheduled = !1);
}
}
function abort(request, reason) {
Expand Down Expand Up @@ -5050,4 +5053,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-1719e6f1";
exports.version = "18.3.0-www-classic-abc7431e";
53 changes: 28 additions & 25 deletions compiled/facebook-www/ReactDOMServer-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -2287,24 +2287,24 @@ function hoistStylesheetDependency(stylesheet) {
function createRenderState(resumableState, generateStaticMarkup) {
var idPrefix = resumableState.idPrefix;
resumableState = idPrefix + "P:";
var JSCompiler_object_inline_segmentPrefix_1557 = idPrefix + "S:";
var JSCompiler_object_inline_segmentPrefix_1561 = idPrefix + "S:";
idPrefix += "B:";
var JSCompiler_object_inline_preconnects_1569 = new Set(),
JSCompiler_object_inline_fontPreloads_1570 = new Set(),
JSCompiler_object_inline_highImagePreloads_1571 = new Set(),
JSCompiler_object_inline_styles_1572 = new Map(),
JSCompiler_object_inline_bootstrapScripts_1573 = new Set(),
JSCompiler_object_inline_scripts_1574 = new Set(),
JSCompiler_object_inline_bulkPreloads_1575 = new Set(),
JSCompiler_object_inline_preloads_1576 = {
var JSCompiler_object_inline_preconnects_1573 = new Set(),
JSCompiler_object_inline_fontPreloads_1574 = new Set(),
JSCompiler_object_inline_highImagePreloads_1575 = new Set(),
JSCompiler_object_inline_styles_1576 = new Map(),
JSCompiler_object_inline_bootstrapScripts_1577 = new Set(),
JSCompiler_object_inline_scripts_1578 = new Set(),
JSCompiler_object_inline_bulkPreloads_1579 = new Set(),
JSCompiler_object_inline_preloads_1580 = {
images: new Map(),
stylesheets: new Map(),
scripts: new Map(),
moduleScripts: new Map()
};
return {
placeholderPrefix: resumableState,
segmentPrefix: JSCompiler_object_inline_segmentPrefix_1557,
segmentPrefix: JSCompiler_object_inline_segmentPrefix_1561,
boundaryPrefix: idPrefix,
startInlineScript: "<script>",
htmlChunks: null,
Expand All @@ -2316,14 +2316,14 @@ function createRenderState(resumableState, generateStaticMarkup) {
importMapChunks: [],
preloadChunks: [],
hoistableChunks: [],
preconnects: JSCompiler_object_inline_preconnects_1569,
fontPreloads: JSCompiler_object_inline_fontPreloads_1570,
highImagePreloads: JSCompiler_object_inline_highImagePreloads_1571,
styles: JSCompiler_object_inline_styles_1572,
bootstrapScripts: JSCompiler_object_inline_bootstrapScripts_1573,
scripts: JSCompiler_object_inline_scripts_1574,
bulkPreloads: JSCompiler_object_inline_bulkPreloads_1575,
preloads: JSCompiler_object_inline_preloads_1576,
preconnects: JSCompiler_object_inline_preconnects_1573,
fontPreloads: JSCompiler_object_inline_fontPreloads_1574,
highImagePreloads: JSCompiler_object_inline_highImagePreloads_1575,
styles: JSCompiler_object_inline_styles_1576,
bootstrapScripts: JSCompiler_object_inline_bootstrapScripts_1577,
scripts: JSCompiler_object_inline_scripts_1578,
bulkPreloads: JSCompiler_object_inline_bulkPreloads_1579,
preloads: JSCompiler_object_inline_preloads_1580,
boundaryResources: null,
stylesToHoist: !1,
generateStaticMarkup: generateStaticMarkup
Expand Down Expand Up @@ -4865,16 +4865,17 @@ function flushCompletedQueues(request, destination) {
0 === request.clientRenderedBoundaries.length &&
0 === request.completedBoundaries.length &&
((request.flushScheduled = !1),
(request = request.resumableState),
request.hasBody &&
(i = request.resumableState),
i.hasBody &&
(destination.push("</"),
destination.push("body"),
destination.push(">")),
request.hasHtml &&
i.hasHtml &&
(destination.push("</"),
destination.push("html"),
destination.push(">")),
destination.push(null));
destination.push(null),
(request.destination = null));
}
}
function enqueueFlush(request) {
Expand All @@ -4883,9 +4884,11 @@ function enqueueFlush(request) {
0 === request.pingedTasks.length &&
null !== request.destination
) {
var destination = request.destination;
request.flushScheduled = !0;
flushCompletedQueues(request, destination);
var destination = request.destination;
destination
? flushCompletedQueues(request, destination)
: (request.flushScheduled = !1);
}
}
function abort(request, reason) {
Expand Down Expand Up @@ -4985,4 +4988,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-4a0e1ba5";
exports.version = "18.3.0-www-modern-75350665";
8 changes: 7 additions & 1 deletion compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -12838,7 +12838,10 @@ function flushCompletedQueues(request, destination) {
}
} // We're done.

close(destination);
close(destination); // We need to stop flowing now because we do not want any async contexts which might call
// float methods to initiate any flushes after this point

stopFlowing(request);
}
}
}
Expand Down Expand Up @@ -12883,6 +12886,9 @@ function startFlowing(request, destination) {
fatalError(request, error);
}
}
function stopFlowing(request) {
request.destination = null;
} // This is called to early terminate a request. It puts all pending boundaries in client rendered state.

function abort(request, reason) {
try {
Expand Down
9 changes: 5 additions & 4 deletions compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -4643,16 +4643,17 @@ function flushCompletedQueues(request, destination) {
0 === request.clientRenderedBoundaries.length &&
0 === request.completedBoundaries.length &&
((request.flushScheduled = !1),
(request = request.resumableState),
request.hasBody &&
(i = request.resumableState),
i.hasBody &&
(writeChunk(destination, "</"),
writeChunk(destination, "body"),
writeChunk(destination, ">")),
request.hasHtml &&
i.hasHtml &&
(writeChunk(destination, "</"),
writeChunk(destination, "html"),
writeChunk(destination, ">")),
(destination.done = !0));
(destination.done = !0),
(request.destination = null));
}
}
function enqueueFlush(request) {
Expand Down

0 comments on commit 24721ff

Please sign in to comment.