Installing BigPipe module along Doubleclick for Publishers (DFP) module causes Drush and Drupal Console to stop working and trowing a ServiceCircularReferenceException:

$ drupal moi dfp
 Installing module(s) "dfp"


 [ERROR] Circular reference detected for service "html_response.attachments_processor.big_pipe", path:
 "html_response.attachments_processor.big_pipe -> html_response.attachments_processor.big_pipe".

Both modules have a service that extend `HtmlResponseAttachmentsProcessor`. If I increase the weight of the BigPipe module before installing DFP, then the error will be:

 [ERROR] Circular reference detected for service "dfp.html_response.attachments_processor", path:
 "dfp.html_response.attachments_processor -> dfp.html_response.attachments_processor".

I've tested against the latest versions of Drupal core (v8.1.x-dev 363b42f32278a7df4583016e6c65043910fbcef2) and DFP (v8.x-1.x-dev 371fe49b745ca37885181bdd833e9976fe1b6986)

I'm not sure if this bug should have been filed in a different place, but assume that it will have more visibility this way.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

SchnWalter created an issue. See original summary.

SchnWalter’s picture

$ drush en dfp
The following extensions will be enabled: dfp
Do you really want to continue? (y/n): y
exception 'Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException' with message 'Circular reference detected for service              [error]
"html_response.attachments_processor.big_pipe", path: "html_response.attachments_processor.big_pipe -> html_response.attachments_processor.big_pipe".' in
/var/www/environment.local/vendor/symfony/dependency-injection/Compiler/CheckCircularReferencesPass.php:67
Stack trace:
#0 /var/www/environment.local/vendor/symfony/dependency-injection/Compiler/CheckCircularReferencesPass.php(70):
Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass->checkOutEdges(Array)
#1 /var/www/environment.local/vendor/symfony/dependency-injection/Compiler/CheckCircularReferencesPass.php(70):
Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass->checkOutEdges(Array)
#2 /var/www/environment.local/vendor/symfony/dependency-injection/Compiler/CheckCircularReferencesPass.php(45):
Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass->checkOutEdges(Array)
#3 /var/www/environment.local/vendor/symfony/dependency-injection/Compiler/Compiler.php(104):
Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass->process(Object(Drupal\Core\DependencyInjection\ContainerBuilder))
#4 /var/www/environment.local/vendor/symfony/dependency-injection/ContainerBuilder.php(597):
Symfony\Component\DependencyInjection\Compiler\Compiler->compile(Object(Drupal\Core\DependencyInjection\ContainerBuilder))
#5 /var/www/environment.local/web/core/lib/Drupal/Core/DrupalKernel.php(1202): Symfony\Component\DependencyInjection\ContainerBuilder->compile()
#6 /var/www/environment.local/web/core/lib/Drupal/Core/DrupalKernel.php(824): Drupal\Core\DrupalKernel->compileContainer()
#7 /var/www/environment.local/web/core/lib/Drupal/Core/DrupalKernel.php(748): Drupal\Core\DrupalKernel->initializeContainer()
#8 /var/www/environment.local/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php(540): Drupal\Core\DrupalKernel->updateModules(Array, Array)
#9 /var/www/environment.local/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php(191): Drupal\Core\Extension\ModuleInstaller->updateKernel(Array)
#10 /var/www/environment.local/web/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php(83): Drupal\Core\Extension\ModuleInstaller->install(Array,
true)
#11 /var/www/environment.local/vendor/drush/drush/commands/core/drupal/environment.inc(131): Drupal\Core\ProxyClass\Extension\ModuleInstaller->install(Array,
true)
#12 /var/www/environment.local/vendor/drush/drush/commands/core/drupal/environment.inc(198): drush_module_install(Array)
#13 /var/www/environment.local/vendor/drush/drush/commands/pm/pm.drush.inc(1167): drush_module_enable(Array)
#14 [internal function]: drush_pm_enable('dfp')
#15 /var/www/environment.local/vendor/drush/drush/includes/command.inc(373): call_user_func_array('drush_pm_enable', Array)
#16 /var/www/environment.local/vendor/drush/drush/includes/command.inc(224): _drush_invoke_hooks(Array, Array)
#17 [internal function]: drush_command('dfp')
#18 /var/www/environment.local/vendor/drush/drush/includes/command.inc(192): call_user_func_array('drush_command', Array)
#19 /var/www/environment.local/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#20 /var/www/environment.local/vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#21 /var/www/environment.local/vendor/drush/drush/drush.php(12): drush_main()
#22 {main}
Fabianx’s picture

Both just use a decorator.

Maybe Symfony cannot handle two services decorating something ...

dawehner’s picture

One cannot use the same decoration_inner_name for each decorator.

Suggestion use: decoration_inner_name: html_response.attachments_processor.original_big_pipe

cilefen’s picture

Priority: Critical » Major
Issue tags: +Contributed project blocker

Contributed project blockers are Major priority.

cilefen’s picture

Or it is just a bug in DFP?

dawehner’s picture

Well, its a conflict between dfp and big_pipe.

Wim Leers’s picture

Assigned: Unassigned » Wim Leers

On this.

Wim Leers’s picture

Title: BigPipe service circular reference exception » BigPipe decorates html_response.attachments_processor, but uses an inner name based on the decorated service, which can cause problems
Assigned: Wim Leers » Unassigned
Priority: Major » Normal
Status: Active » Needs review
Issue tags: -Contributed project blocker
FileSize
1.2 KB

As per usual, @dawehner is right:

One cannot use the same decoration_inner_name for each decorator.

Suggestion use: decoration_inner_name: html_response.attachments_processor.original_big_pipe

This is exactly right. See https://symfony.com/doc/current/service_container/service_decoration.html:

The generated inner id is based on the id of the decorator service (app.decorating_mailer here), not of the decorated service (app.mailer here). This is mandatory to allow several decorators on the same service (they need to have different generated inner ids).

So the solution is simple, and can and should in fact be adopted by both BigPipe in Drupal 8 core and by the dfp module:

     class: \Drupal\big_pipe\Render\BigPipeResponseAttachmentsProcessor
     decorates: html_response.attachments_processor
-    decoration_inner_name: html_response.attachments_processor.original
-    arguments: ['@html_response.attachments_processor.original', '@asset.resolver', '@config.factory', '@asset.css.collection_renderer', '@asset.js.collection_renderer', '@request_stack', '@renderer', '@module_handler']
+    decoration_inner_name: html_response.attachments_processor.decorated_by_big_pipe
+    arguments: ['@html_response.attachments_processor.decorated_by_big_pipe', '@asset.resolver', '@config.factory', '@asset.css.collection_renderer', '@asset.js.collection_renderer', '@request_stack', '@renderer', '@module_handler']

If either module fixes this, it's fixed. So this patch does not block dfp; dfp can fix this itself. Nevertheless, Drupal core should be fixed too.

Wim Leers’s picture

FileSize
1.11 KB

Alternatively, we don't specify decoration_inner_name at all, and just rely on the auto-generated inner name. Which is probably the simpler/easier thing to do:

     class: \Drupal\big_pipe\Render\BigPipeResponseAttachmentsProcessor
     decorates: html_response.attachments_processor
-    decoration_inner_name: html_response.attachments_processor.original
-    arguments: ['@html_response.attachments_processor.original', '@asset.resolver', '@config.factory', '@asset.css.collection_renderer', '@asset.js.collection_renderer', '@request_stack', '@renderer', '@module_handler']
+    arguments: ['@html_response.attachments_processor.big_pipe.inner', '@asset.resolver', '@config.factory', '@asset.css.collection_renderer', '@asset.js.collection_renderer', '@request_stack', '@renderer', '@module_handler']
dawehner’s picture

@Wim Leers
Did you actually tried out what happens when you do that?
The code looks like this:

            $renameId = isset($service['decoration_inner_name']) ? $service['decoration_inner_name'] : null;
            $priority = isset($service['decoration_priority']) ? $service['decoration_priority'] : 0;
            $definition->setDecoratedService($service['decorates'], $renameId, $priority);

which seems more likely to cause some conflict.

Wim Leers’s picture

Did you actually tried out what happens when you do that?

Yes. I tested both patches.

Both #9 and #10 work fine. In either case, the decorated service is renamed to something unique that contains the decorating service's name, which ensures uniqueness. Just as https://symfony.com/doc/current/service_container/service_decoration.html prescribes.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Gotcha.
Yeah I guess this is happening somewhere else than in the code quoted above.

SchnWalter’s picture

Thank you very much, the latest patch (#10) works perfectly!

Wim Leers’s picture

Glad to see that confirmed by you :)

Fabianx’s picture

RTBC + 1, looks good to me.

  • catch committed b9aa30a on 8.3.x
    Issue #2792817 by Wim Leers, dawehner: BigPipe decorates html_response....

  • catch committed 926a434 on 8.2.x
    Issue #2792817 by Wim Leers, dawehner: BigPipe decorates html_response....
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

Wim Leers’s picture

Thanks!

Status: Fixed » Closed (fixed)

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

bleen’s picture