Use tagged iterators in CacheTagsInvalidator

Small comment feel free to self RTBC if it's nothing or you delete. @longwave.

FYI apparently this circular reference issue can be reproduced with core syslog module, which loads configuration in its logger constructor, making this both a bug and a task.

The syslog issue is not related to this and has an existing open issue.

+1 to this, noticed as well that all these bins get initialized early a while ago.

Not sure about BC here. This is a new constructor method. What happens if someone subclasses with it's own construct but didn't call the parent?

There is one subclass in contrib that I found, but that doesn't have a constructor: https://search.tresbien.tech/search?q=%22extends%20CacheTagsInvalidator%22

@berdir: This issue's merge request resolves the circular reference when core syslog and symfony_mailer modules are installed:

./vendor/bin/drush cr

In Container.php line 143:
                                                                                                                                                                                      
  Circular reference detected for service "logger.channel.default", path: "Drupal\Core\Recipe\Command\RecipeCommand -> logger.channel.default -> logger.factory -> logger.syslog ->   
  Drupal\menu_link_content\Hook\MenuLinkContentHooks -> plugin.manager.menu.link -> Drupal\Core\Menu\MenuTreeStorageInterface -> cache_tags.invalidator -> plugin.manager.block".

So as far as I can tell, the syslog issue is directly related to this one?

Note that I only get the circular reference every other time I run drush cr, not every time

@mfb: That looks super weird to me, especially:

 logger.syslog -> Drupal\menu_link_content\Hook\MenuLinkContentHooks

Why would MenuLinkContentHooks get injected into logger.syslog, that makes no sense? Hook services should never get injected into something, especially not low-level services like a logger.

Also, plugin.manager.block shouldn't get injected into cache_tags.invalidator, that doesn't implement this tag, not in core at least. Maybe you're using some kind of custom module that overrides that?

To be clear, I fully support this change. I also think log channels should pretty much always use AutowireServiceClosure although I'm not sure would help much in this case. It's just quite surprising that this fixes that problem on your site and it looks like there might be something very strange going on there.

@longwave is one of several bug reporters who brought this issue to my attention. I thought it was "weird" too - I merely reproduced it.

It requires only the default install profile of Drupal core, core syslog module, symfony_mailer module, and Drush.

What I'd do is put a breakpoint in the constructor of MenuLinkContentHooks and then look at the backtrace. what exactly creates that, where, which parameter to what other service is it? I suspect the container recursion check is a bit confused, maybe something triggers a hook, possibly entity type definitions if that's not been built yet at that point?

You've been active in #3103620: Dependency on config storage causes circular reference in service container too, that's the issue I was thinking off. Would be nice to resolve that one.

According to @longwave, it's because "Symfony Mailer has a config factory override that also performs plugin discovery". As a result of those other subsystems pulling in a logger, any logger that gets configuration in its constructor, such as core syslog module, can trigger the circular reference when symfony_mailer is installed.

@longwave: I think I'm OK keeping it as is. On BC, I suppose that's a committer decision, technically the constructor and property type changes are BC breaks, also the fact that there's no one more list of services to worry about in case someone did replace it completely, I found only the one implementation in contrib that should be OK with those changes. I can mark it as RTBC with a note on that if you create a CR for that.