Loop affected by modifications to the list it iterates through

[greiner]

Environment

Chrome 70
Adblock Plus 3.3.2

How to reproduce

Iterate through filters using SpecialSubscription.filters() while calling filterStorage.removeFilter() for some of the filters within the loop.

The same issue may also apply to other generators.

Observed behaviour

Loop skips some of the filters.

Expected behaviour

Loop doesn't skip any of the filters.

Further information

This issue was discovered in ​this UI merge request relating to ​this Core change.

[agiammarchi]

Thanlks for putting me in Cc Thomas, FWIW I think if we need to consume generators all at once to avoid possible side effects while working with these we are missing any benefit of having generators, keeping only the CPU and RAM overhead.

Wouldn't be possible to queue any async operation so that a for/of could still modify all yeald values without missing any of them due Array modification behind the scene?

This is IMO not super urgent but we need to code aware that each for/of should be performed over Array.from(iterable) and not directly via the iterable.

Any improvement to this would be welcome.

[mjethani]

We'll need to rewrite the filterStorage.removeFilter() function at some point, but meanwhile I think the semantics of Subscription.filters() should be similar to how it works for Set objects, i.e. items deleted do not cause any other items to be skipped.

In the meanwhile, if you would like to go ahead with the UI changes, I would recommend to use Subscription.filterCount and Subscription.filterAt(index) to iterate backwards by index as before. When this issue is fixed, you could then change it again.

[greiner]

Thanks for the tip. I don't mind changing it again later so I've ​updated the merge request for the UI changes to use those two.

[sebastian]

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the ​new issue tracker.