@samitouri / QOS-React / commits / 4dabdd2a30

Limit the number of nested synchronous updates (#10178)

* Limit the number of nested synchronous updates In Stack, an infinite update loop would result in a stack overflow. This gives the same behavior to Fiber. Conceptually, I think this check belongs in findNextUnitOfWork, since that is what we call right before starting a new stack. I've put it in scheduleUpdate for now so I have access to the component that triggered the nested update. But we could track that explicitly instead. I've chosen 1000 as the limit rather arbitrarily. Most legit use cases should really have a much smaller limit, but a smaller limit could break existing code. For now I'm only concerned with preventing an infinite loop. We could add a warning that fires at the smaller limit. * Move check to findNextUnitOfWork Including the name of the component in the message probably isn't necessary. The JS stack will include either componentDidUpdate or componentWillUpdate. And the component that's updating won't necessarily be the component whose lifecycle triggered it. So let's move the infinite loop check to findNextUnitWork as I originally wanted to.

Andrew Clark committed Jul 13, 2017 at 15:56 UTC 4dabdd2a3048b5883cc71f8c91442a905c838b6d

3 files changed +43

scripts/fiber/tests-passing.txt

@@ -596,6 +596,7 @@ src/renderers/__tests__/ReactUpdates-test.js

596 * mounts and unmounts are sync even in a batch

597 * does not re-render if state update is null

598 * synchronously renders hidden subtrees

599 +* does not fall into an infinite update loop

600

601 src/renderers/__tests__/multiple-copies-of-react-test.js

602 * throws the "Refs must have owner" warning

src/renderers/__tests__/ReactUpdates-test.js

+20

@@ -1138,4 +1138,24 @@ describe('ReactUpdates', () => {

1138 ReactDOM.render(<Foo />, container);

1139 expect(ops).toEqual(['Foo', 'Bar', 'Baz']);

1140 });

1141 +

1142 + it('does not fall into an infinite update loop', () => {

1143 + class NonTerminating extends React.Component {

1144 + state = {step: 0};

1145 + componentDidMount() {

1146 + this.setState({step: 1});

1147 + }

1148 + componentWillUpdate() {

1149 + this.setState({step: 2});

1150 + }

1151 + render() {

1152 + return <div>Hello {this.props.name}{this.state.step}</div>;

1153 + }

1154 + }

1155 +

1156 + const container = document.createElement('div');

1157 + expect(() => {

1158 + ReactDOM.render(<NonTerminating />, container);

1159 + }).toThrow('Maximum');

1160 + });

1161 });

src/renderers/shared/fiber/ReactFiberScheduler.js

+22

@@ -231,6 +231,11 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

231 let isCommitting: boolean = false;

232 let isUnmounting: boolean = false;

233

234 + // Use these to prevent an infinite loop of nested updates

235 + let didCommit = false;

236 + let nestedSyncUpdates = 0;

237 + let NESTED_SYNC_UPDATE_LIMIT = 1000;

238 +

239 function resetContextStack() {

240 // Reset the stack

241 reset();

@@ -285,6 +290,20 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

290 nextPriorityLevel = highestPriorityLevel;

291 priorityContext = nextPriorityLevel;

292

293 + if (

294 + didCommit &&

295 + (nextPriorityLevel === TaskPriority ||

296 + nextPriorityLevel === SynchronousPriority)

297 + ) {

298 + invariant(

299 + nestedSyncUpdates++ <= NESTED_SYNC_UPDATE_LIMIT,

300 + 'Maximum update depth exceeded. This can happen when a ' +

301 + 'component repeatedly calls setState inside componentWillUpdate or ' +

302 + 'componentDidUpdate. React limits the number of nested updates to ' +

303 + 'prevent infinite loops.',

304 + );

305 + }

306 +

307 // Before we start any new work, let's make sure that we have a fresh

308 // stack to work from.

309 // TODO: This call is buried a bit too deep. It would be nice to have

@@ -415,6 +434,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

434 // local to this function is because errors that occur during cWU are

435 // captured elsewhere, to prevent the unmount from being interrupted.

436 isCommitting = true;

437 + didCommit = true;

438 if (__DEV__) {

439 startCommitTimer();

440 }

@@ -991,6 +1011,8 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

1011 firstUncaughtError = null;

1012 capturedErrors = null;

1013 failedBoundaries = null;

1014 + didCommit = false;

1015 + nestedSyncUpdates = 0;

1016 if (__DEV__) {

1017 stopWorkLoopTimer();

1018 }