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

  1. Install the Cas mock server.
  2. Install Config Ignore 3.0-beta1
  3. Instantiate the ServerManager service in scope of a fully bootstrapped Drupal kernel, e.g. by executing our Behat scenario hook CasMockServerContext::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.

CommentFileSizeAuthor
#5 interdiff.txt546 bytespfrenssen
#5 3231532-5.patch2.52 KBpfrenssen
#2 3231532-2.patch1.99 KBpfrenssen

Comments

pfrenssen created an issue. See original summary.

pfrenssen’s picture

Status: Active » Needs review
StatusFileSize
new1.99 KB

This patch circumvents the possible circular dependency by sideloading the cache tags invalidator service.

claudiu.cristea’s picture

Status: Needs review » Reviewed & tested by the community

LGTM

claudiu.cristea’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/src/ServerManager.php
@@ -25,24 +24,14 @@ class ServerManager implements ServerManagerInterface {
-  public function __construct(StateInterface $state, CacheTagsInvalidatorInterface $cacheTagsInvalidator) {
+  public function __construct(StateInterface $state) {

Should we remove if from service definition too?

pfrenssen’s picture

Status: Needs work » Needs review
StatusFileSize
new2.52 KB
new546 bytes

Ah yes indeed!

claudiu.cristea’s picture

Status: Needs review » Reviewed & tested by the community

  • pfrenssen committed cb18369 on 8.x-1.x
    Issue #3231532 by pfrenssen, claudiu.cristea: Potential circular...
pfrenssen’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.