[patch] [cla] Subscribing to on.Evented events from event handler

[Mark Wubben]

Use case:

• I have an on.Evented instance which emits an event
• Event handler A removes itself before subscribing a new event handler B

Because the code path for this use case uses after advice, if there is another after advice C to be run after my event handler A, the following occurs:

• Event handler A removes itself successfully
• When it subscribes the new event handler B, it's chained from the other after advice C (as a next)
• After the other after advice C has executed, the new event handler B is run

Unfortunately the other after advice C also removes itself before adding a new event handler D, meaning I'm now caught in an infinite loop, i.e. A -> C -> B -> D -> C' -> D' -> …

The underlying problem is that if you add a new after advice, while after advice is already executing, and at least one other advice is next in line, the new after advice will be run within the same execution. This can lead to infinite loops. Not great for aspects, but that could be considered a design feature. It makes no sense for event handlers though.

This patch changes dojo/aspect to not execute new after advice if after advice is currently being executed: <​https://github.com/EqualMedia/dojo/commit/16ecc7796a0988076d588c6a87903a14bc20da80>

[Chris Mitchell]

We're also encountering this issue in several projects. What's the chance of getting this patch into 1.7.2?

[Chris Mitchell]

We have also verified that Mark W's patch fixes the problems on our end.

[bill]

It's a little unfortunate to add all that code, although I can appreciate the corner-case markwubben describes.

I wonder if it would be shorter to flag the last advice in the chain at the point that after() started executing it. Or in other words, if you add new advice while after() is running, temporarily mark the new advices with a "new" flag. Either way it sounds like tricky code.

[Chris Mitchell]

Bill, thanks for taking a look at this.

Another improvement to Mark's patch (from Oliver Rau/IBM CCLA) is

195 +          if(dispatcher.afterAdviseQueue.length){

dispatcher.afterAdviseQueue can be null / undefined here. so changing the if to

195 +          if(dispatcher.afterAdviseQueue && dispatcher.afterAdviseQueue.length){

this corner case is being hit by a number of our teams now as they move to 1.7 eventing

[Kris Zyp]

In [27744]:

Only execute after aspects that were added before the current dispatch execution began, fixes #13743 !strict

[Kris Zyp]

In [27770]:

Only execute after aspects that were added before the current dispatch execution began, applied to 1.7, refs #13743 !strict

[bill]

No milestone specified, bulk update to 1.8.