Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change detection error -> infinite loop #4323

Closed
alxhub opened this issue Sep 22, 2015 · 3 comments
Closed

Change detection error -> infinite loop #4323

alxhub opened this issue Sep 22, 2015 · 3 comments

Comments

@alxhub
Copy link
Member

alxhub commented Sep 22, 2015

I found a case where change detection enters an infinite loop on an error.

The code in question:

@Component({selector: "signup-comp"})
@View({
  directives: [CORE_DIRECTIVES, FORM_DIRECTIVES],
  template: `
    <form #f="form" (ng-submit)='onSignUp(f.value)'>
      <div ng-control-group='credentials' #credentials="form">
        Login <input type='text' ng-control='login'>
        Password <input type='password' ng-control='password'>
      </div>
      <div *ng-if="!d.valid">Credentials are invalid</div>
      <div ng-control-group='personal'>
        Name <input type='text' ng-control='name'>
      </div>
      <button type='submit'>Sign Up!</button>
    </form>
  `
})

The key is the ng-if expression !d.valid. d is not defined, and this triggers an infinite loop.

Here is a full Plunker example.

@alxhub
Copy link
Member Author

alxhub commented Sep 23, 2015

It seems like <div ng-control-group='credentials'></div> combined with the ng-if is enough to trigger the bug.

@alxhub
Copy link
Member Author

alxhub commented Sep 29, 2015

I tracked down the root cause:

NgControlGroup declares an onInit method, which schedules some work to be completed asynchronously, via PromiseWrapper.resolve.

The problem is that onInit can actually be called multiple times - it is called if change detection runs and has never completed successfully. If an exception happens during change detection after onInit is called, AbstractChangeDetector.alreadyChecked will not be set and onInit will be run again during the next tick. Since onInit in NgControlGroup schedules asynchronous work, that next tick will happen immediately, causing a crash loop.

@alxhub alxhub self-assigned this Oct 1, 2015
@alxhub alxhub added this to the beta-00 milestone Oct 27, 2015
alxhub added a commit to alxhub/angular that referenced this issue Oct 27, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 27, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 27, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 27, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 27, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 27, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 28, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 28, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 28, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit to alxhub/angular that referenced this issue Oct 29, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue angular#4323.
alxhub added a commit that referenced this issue Oct 29, 2015
…ndings or lifecycle events throw exceptions and prevents further detection.

- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue #4323.

Closes #4953
@alxhub alxhub closed this as completed Nov 2, 2015
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants