Manually calling $digest on a scope prevents $rootScope.$digest from executing for pending $evalAsyncs

[BrandonLWhite]

Issue

This is a bug report for an intermittent issue we are seeing where due to specific timing of asynchronous callbacks executing causes $rootScope.$digest from being called when it is expected that Angular should otherwise do so.

I have a plunkr that simulates the scenario to induce the failure every time.

The current behavior is that a controller "A" scope is not digested upon conclusion of a $.then when another controller "B" scope's $digest is called manually.

This plunkr demonstrates the issue. Please follow these steps:

1. Click "Test (Bugged)"
2. Observe that ctrl.result changes to "Started" but does not end with "Finished". Nor does ctrl.shared.foo display the same value as ctrl2.shared.foo.

The expected behavior is that ctrl.result ends displaying "Finished" and ctrl.shared.foo matches what is shown for ctrl2.shared.foo.

What is being demonstrated is the $digest for ctrl2 causes the digest for ctrl to be skipped. Because ctrl is using $q the expectation is that its scope will be digested at the conclusion of the $q.then function (presumably as a result of a $rootScope.$digest being performed by Angular).

• After clicking "Reset", click the "Test (Workaround)" to see the test using a hack we are currently using in some of our code to workaround the issue. Note how the values displayed for ctrl are as expected.
  This workaround (undesirably) inspects $scope.$$asyncQueue.length and if there are any values, it avoids the manual call to $scope.$digest. This permits Angular's deferred call to process the queue and rootScope.digest to later run, and everything is digested as expected.

It appears that all browsers and all "recent" versions of Angular exhibit the issue.

What is going on?

In our application, here is what's going on:

• Controller A and B are servicing completely separate parts of the UI/DOM. Their scopes are in different branches.
• Controller A performs an asynchronous operation (outside of angular) which it wraps in a $q.when. When the data becomes available, the corresponding $q.then is called where we load up the controller's model and then we expect Angular to digest the changes and update the DOM.
• Controller B likewise is performing an asynchronous operation (via a service actually, but that's irrelevant to honing in on the issue). However, it is not using $q and instead performs a manual call to $scope.$digest() on its own scope branch due to performance reasons. The motivation for this is an optimization to prevent digesting the entire app for something that is happening extremely frequently and affects just a tiny portion of the UI.
• Normally these two completely unrelated things work just fine, and we get what we are after with regard to Controller B's performance impact.
• However, on some occasions typically when the browser gets busy for a bit, Controller B's $digest stomps on Controller A's digest.

Tracing through Angular here is what I've come up with:

1. Controller A's async result comes in and corresponding callback invocation (which involves $q.then) is in the browser's dispatch queue. At some point just before this callback being invoked Controller B's async result callback is pushed to the queue. This is the key to seeing the failure -- you need both callbacks to be in the browser's queue.
2. Controller A's callback is then called as expected. Angular adds the .then call to the $$asyncQueue and defers a call to flush the $$asyncQueue and do a $rootScope.$digest.
3. Controller B's callback is then called. Controller B's scope.$digest is called. It sees that the $$asyncQueue has stuff to do, so it does it -- without a $rootScope.$digest.
4. The deferred call from step 2 executes, sees that asyncQueue is empty, and so it doesn't do anything.

It seems like scope.$digest shouldn't be processing the asyncQueue like this unless the scope being digested is the rootScope.

[BrandonLWhite]

This test case is a tad bit simpler http://plnkr.co/edit/Clqp1Y It doesn't involve $q and calls $scope.$evalAsync directly.

[dcherman]

Was thinking about this earlier since digesting just a single branch can be a sweet performance optimization, but not having the the $evalAsync queue drained by $digest is potentially unsafe. While it's not documented as such since no guarantees are made as to when the expression is evaluated, I wonder how much people have been treating it like a nextTick function. Even if we didn't drain the queue, we would still need to $apply after a tick of the event loop in order to have things work.

Gist of the problem is that $evalAsync handlers could be doing anything, including modifying parent state either directly, via two-way bindings, or by modifying a service that a parent scope is watching. Given that, there's not much choice other than to require a full $apply with the current state of things. Even if you split up the $evalAsync queue into a queue per scope so that you only drain your own, the same problem would still exist.

[gkalpak]

Even if you split up the $evalAsync queue into a queue per scope so that you only drain your own, the same problem would still exist.

I don't think that is a problem -unless I misunderstood what you mean. If you $digest locally, you should accept the fact that only the current scope sub-tree wll be affected.

Having separate async queues would be ideal, if we could still guarantee the correct order of execution (which would get too complicated - I think).

(Disclaimer: Haven't thought this through yet.)

[dcherman]

If you control the entire subtree, then you can definitely maintain unidirectional dataflow to make this work. The problem is that a blanket statement like this would break composability/isolation for components; you couldn't safely call $digest without also knowing how every directive beneath you is implemented. Granted, apparently you can't do that today either.

[dcherman]

For reference, each scope did used to have its own execution queue, but it was changed way back in 1.1.1

331cd5a

Reverting to that model would break this optimization, or at least make it more difficult to accomplish as we need to know whether or not child scopes have pending tasks to execute either through visiting the scope, or via a counter that gets incremented/decremented.

https://github.com/angular/angular.js/blob/master/src/ng/rootScope.js#L824-L828

[gkalpak]

Interesting! So, going back to multiple queues is not worth it, since it would break other usecases as mentioned in the commit message of 331cd5a.

It would be nice to fix OP's usecase though. (No idea what it could be atm.)

[samal-rasmussen]

Isn't the workaround to use $applyAsync instead of $evalAsync? Don't $applyAsync and $evalAsync use separate queues, and the local child scope $digest call will only empty every $evalAsync queue?

It seems to work. I only changed $evalAsync to $applyAsync: http://plnkr.co/edit/9deFSvmLs2tJEtgsfNkr?p=preview

[gkalpak]

Yes, this would work, but it doesn't help in this usecase where something is using $evalAsync under the hood (and the user has no way to change that).

Back to the problem at hand, a simpler solution might be calling $rootScope.$digest(), even if asyncQueue is empty. There might be one extra $digest is some corner cases, but I doubt it would break anyone.

[BrandonLWhite]

@samal84 Could you please elaborate on what you are suggesting?

If you are implying that one could simply forego $q/$evalAsync in their code in order to avoid this issue then that is not really feasible. In the repro app someService.shared.otherAsyncOperation mimics any code that makes use of $q or $evalAsync. That code being yours or any 3rd party code, even Angular's stuff like $http (I think). So you really cannot escape this.

[dcherman]

@gkalpak Call $rootScope.$digest() when?

[samal-rasmussen]

@BrandonLWhite @gkalpak Yes my workaround only works when you have control over where $evalAsync/$applyAsync gets called.

[gkalpak]

@gkalpak Call $rootScope.$digest() when?

$evalAsync results in a deferred task that looks something like this:

if (asyncQueue.length) {
  $rootScope.$digest();
}

We could remove the asyncQueue.length check, so even if the queue was processed by another scope, the changes will propagate to all scopes (via $rootScope.$digest()).

[dcherman]

So while that would work as an easy fix, you end up running potentially needless digest cycles. While that wouldn't break anyone, running a full $digest cycle is definitely not an inexpensive operation, especially when one may already have been run via $onChanges since that's how it's implemented internally (which in itself makes calling $digest as an optimization much, much more difficult).

This also breaks the optimization since you'll no longer be able to call $digest and know exactly what part of the scope heirarchy you're going to digest, but you could do target = asyncQueue.length ? $rootScope : this. That would still fail in the case that someone calls $evalAsync internally that affects something outside of the digested scope heirarchy. One solution to that could be that since we know the id of each scope having $evalAsync called, we could use a simple object to track whether or not that particular scope was digested if the original target was not the $rootScope since by definition, all child scopes will be digested in that case. That way, we avoid the extra property lookups that this would entail in the common case of using $apply.

[gkalpak]

I would rather break the optimization than have broken behavior. But I agree that potentially running an extra digest (even if it will only affect some cases) isn't ideal. We should come up with a more appropriate solution.