Fix react-dom/server context leaks when render stream destroyed early (#14706)
* Fix react-dom/server context memory retention * Test for pollution of later renders * Inline loop * More tests
overlookmotel committed
Feb 14, 2019 at 19:50 UTC
ab7a67b1dc0e44bf74545ccf51a8c143b3af7402
2 files changed
+103
packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js
+93
@@ -482,5 +482,98 @@ describe('ReactDOMServerIntegration', () => {
482
);
483
}
484
});
485
+
486
+ // Regression test for https://github.com/facebook/react/issues/14705
487
+ it('does not pollute later renders when stream destroyed', () => {
488
+ const LoggedInUser = React.createContext('default');
489
+
490
+ const AppWithUser = user => (
491
+ <LoggedInUser.Provider value={user}>
492
+ <header>
493
+ <LoggedInUser.Consumer>{whoAmI => whoAmI}</LoggedInUser.Consumer>
494
+ </header>
495
+ </LoggedInUser.Provider>
496
+ );
497
+
498
+ const stream = ReactDOMServer.renderToNodeStream(
499
+ AppWithUser('Amy'),
500
+ ).setEncoding('utf8');
501
+
502
+ // This is an implementation detail because we test a memory leak
503
+ const {threadID} = stream.partialRenderer;
504
+
505
+ // Read enough to render Provider but not enough for it to be exited
506
+ stream._read(10);
507
+ expect(LoggedInUser[threadID]).toBe('Amy');
508
+
509
+ stream.destroy();
510
+
511
+ const AppWithUserNoProvider = () => (
512
+ <LoggedInUser.Consumer>{whoAmI => whoAmI}</LoggedInUser.Consumer>
513
+ );
514
+
515
+ const stream2 = ReactDOMServer.renderToNodeStream(
516
+ AppWithUserNoProvider(),
517
+ ).setEncoding('utf8');
518
+
519
+ // Sanity check to ensure 2nd render has same threadID as 1st render,
520
+ // otherwise this test is not testing what it's meant to
521
+ expect(stream2.partialRenderer.threadID).toBe(threadID);
522
+
523
+ const markup = stream2.read(Infinity);
524
+
525
+ expect(markup).toBe('default');
526
+ });
527
+
528
+ // Regression test for https://github.com/facebook/react/issues/14705
529
+ it('frees context value reference when stream destroyed', () => {
530
+ const LoggedInUser = React.createContext('default');
531
+
532
+ const AppWithUser = user => (
533
+ <LoggedInUser.Provider value={user}>
534
+ <header>
535
+ <LoggedInUser.Consumer>{whoAmI => whoAmI}</LoggedInUser.Consumer>
536
+ </header>
537
+ </LoggedInUser.Provider>
538
+ );
539
+
540
+ const stream = ReactDOMServer.renderToNodeStream(
541
+ AppWithUser('Amy'),
542
+ ).setEncoding('utf8');
543
+
544
+ // This is an implementation detail because we test a memory leak
545
+ const {threadID} = stream.partialRenderer;
546
+
547
+ // Read enough to render Provider but not enough for it to be exited
548
+ stream._read(10);
549
+ expect(LoggedInUser[threadID]).toBe('Amy');
550
+
551
+ stream.destroy();
552
+ expect(LoggedInUser[threadID]).toBe('default');
553
+ });
554
+
555
+ it('does not pollute sync renders after an error', () => {
556
+ const LoggedInUser = React.createContext('default');
557
+ const Crash = () => {
558
+ throw new Error('Boo!');
559
+ };
560
+ const AppWithUser = user => (
561
+ <LoggedInUser.Provider value={user}>
562
+ <LoggedInUser.Consumer>{whoAmI => whoAmI}</LoggedInUser.Consumer>
563
+ <Crash />
564
+ </LoggedInUser.Provider>
565
+ );
566
+
567
+ expect(() => {
568
+ ReactDOMServer.renderToString(AppWithUser('Casper'));
569
+ }).toThrow('Boo');
570
+
571
+ // Should not report a value from failed render
572
+ expect(
573
+ ReactDOMServer.renderToString(
574
+ <LoggedInUser.Consumer>{whoAmI => whoAmI}</LoggedInUser.Consumer>,
575
+ ),
576
+ ).toBe('default');
577
+ });
578
});
579
});
packages/react-dom/src/server/ReactPartialRenderer.js
+10
@@ -715,6 +715,7 @@ class ReactDOMServerRenderer {
715
destroy() {
716
if (!this.exhausted) {
717
this.exhausted = true;
718
+ this.clearProviders();
719
freeThreadID(this.threadID);
720
}
721
}
@@ -776,6 +777,15 @@ class ReactDOMServerRenderer {
777
context[this.threadID] = previousValue;
778
}
779
780
+ clearProviders(): void {
781
+ // Restore any remaining providers on the stack to previous values
782
+ for (let index = this.contextIndex; index >= 0; index--) {
783
+ const context: ReactContext<any> = this.contextStack[index];
784
+ const previousValue = this.contextValueStack[index];
785
+ context[this.threadID] = previousValue;
786
+ }
787
+ }
788
+
789
read(bytes: number): string | null {
790
if (this.exhausted) {
791
return null;