Opened on 12/03/2018 at 01:06:07 PM

Closed on 08/29/2019 at 05:43:52 PM

#7152 closed defect (rejected)

Loop affected by modifications to the list it iterates through

Reported by: greiner Assignee:
Priority: Unknown Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: mjethani, agiammarchi, jsonesen Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):



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.

Attachments (0)

Change History (5)

comment:1 Changed on 12/03/2018 at 01:10:45 PM by 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.

comment:2 Changed on 12/03/2018 at 02:48:04 PM by 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.

comment:3 Changed on 12/03/2018 at 03:38:53 PM by 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.

comment:4 Changed on 03/17/2019 at 10:16:51 AM by mjethani

  • Cc jsonesen added

comment:5 Changed on 08/29/2019 at 05:43:52 PM by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from new to closed

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

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
Note: See TracTickets for help on using tickets.