Problem/Motivation
There is a potential circular dependency between ServerManager and CasMockServerConfigOverriderTest. They do not directly depend on each other, but the circular dependency can be triggered by third party cache tag invalidator services.
I noticed this in the wild when I updated the Config Ignore module to the latest 3.0-beta1 release. In this release a new cache tag invalidator is introduced which depends on the ConfigFactory service. ConfigFactory will include our CasMockServerConfigOverrider in the dependency chain, and this depends on our ServerManager so it can check if the server is running before it applies the config override. The ServerManager in turn depends on the CacheTagsInvalidator so it can clear caches whenever the server is started or stopped. This causes an endless loop and the following exception is thrown:
Circular reference detected for service "cas_mock_server.server_manager", path: "cas_mock_server.server_manager -> cache_tags.invalidator -> config_ignore.event_subscriber -> config.factory -> cas_mock_server.config_overrider". (Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException)
Steps to reproduce
- Install the Cas mock server.
- Install Config Ignore 3.0-beta1
- Instantiate the
ServerManagerservice in scope of a fully bootstrapped Drupal kernel, e.g. by executing our Behat scenario hookCasMockServerContext::startMockServer().
Proposed resolution
A simple solution would be to side load the cache tags invalidator service on demand, rather than declaring it as a fixed dependency in ServerManager.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | interdiff.txt | 546 bytes | pfrenssen |
| #5 | 3231532-5.patch | 2.52 KB | pfrenssen |
Comments
Comment #2
pfrenssenThis patch circumvents the possible circular dependency by sideloading the cache tags invalidator service.
Comment #3
claudiu.cristeaLGTM
Comment #4
claudiu.cristeaShould we remove if from service definition too?
Comment #5
pfrenssenAh yes indeed!
Comment #6
claudiu.cristeaComment #8
pfrenssen