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.
This commit is contained in:
Andrew Clark
2017-07-13 15:56:02 -07:00
committed by GitHub
parent 0e08422e4f
commit 92cbbfd8bf
3 changed files with 43 additions and 0 deletions
+1
View File
@@ -596,6 +596,7 @@ src/renderers/__tests__/ReactUpdates-test.js
* mounts and unmounts are sync even in a batch
* does not re-render if state update is null
* synchronously renders hidden subtrees
* does not fall into an infinite update loop
src/renderers/__tests__/multiple-copies-of-react-test.js
* throws the "Refs must have owner" warning
@@ -1138,4 +1138,24 @@ describe('ReactUpdates', () => {
ReactDOM.render(<Foo />, container);
expect(ops).toEqual(['Foo', 'Bar', 'Baz']);
});
it('does not fall into an infinite update loop', () => {
class NonTerminating extends React.Component {
state = {step: 0};
componentDidMount() {
this.setState({step: 1});
}
componentWillUpdate() {
this.setState({step: 2});
}
render() {
return <div>Hello {this.props.name}{this.state.step}</div>;
}
}
const container = document.createElement('div');
expect(() => {
ReactDOM.render(<NonTerminating />, container);
}).toThrow('Maximum');
});
});
@@ -231,6 +231,11 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
let isCommitting: boolean = false;
let isUnmounting: boolean = false;
// Use these to prevent an infinite loop of nested updates
let didCommit = false;
let nestedSyncUpdates = 0;
let NESTED_SYNC_UPDATE_LIMIT = 1000;
function resetContextStack() {
// Reset the stack
reset();
@@ -285,6 +290,20 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
nextPriorityLevel = highestPriorityLevel;
priorityContext = nextPriorityLevel;
if (
didCommit &&
(nextPriorityLevel === TaskPriority ||
nextPriorityLevel === SynchronousPriority)
) {
invariant(
nestedSyncUpdates++ <= NESTED_SYNC_UPDATE_LIMIT,
'Maximum update depth exceeded. This can happen when a ' +
'component repeatedly calls setState inside componentWillUpdate or ' +
'componentDidUpdate. React limits the number of nested updates to ' +
'prevent infinite loops.',
);
}
// Before we start any new work, let's make sure that we have a fresh
// stack to work from.
// 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>(
// local to this function is because errors that occur during cWU are
// captured elsewhere, to prevent the unmount from being interrupted.
isCommitting = true;
didCommit = true;
if (__DEV__) {
startCommitTimer();
}
@@ -991,6 +1011,8 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
firstUncaughtError = null;
capturedErrors = null;
failedBoundaries = null;
didCommit = false;
nestedSyncUpdates = 0;
if (__DEV__) {
stopWorkLoopTimer();
}