Core uses a compile pass (defined in DependencySerializationTraitPass) to attach a property (_serviceId) to all defined services.

The Symfony Service Container allows decorating a service (http://symfony.com/doc/current/components/dependency_injection/advanced....):

  webprofiler.debug.config.factory:
    class: Drupal\webprofiler\Config\ConfigFactoryWrapper
    public: false
    decorates: config.factory
    arguments: ['@webprofiler.debug.config.factory.inner', '@webprofiler.config']

but the DependencySerializationTraitPass class fails to set the property _serivceId on the correct service:

protected function getConfig_FactoryService()
{
  $a = new \Drupal\Core\Config\ConfigFactory($this->get('config.storage'), $this->get('webprofiler.debug.event_dispatcher'), $this->get('config.typed'));
  $a->_serviceId = 'config.factory';

  return $this->services['config.factory'] = new \Drupal\webprofiler\Config\ConfigFactoryWrapper($a, $this->get('webprofiler.config'));
}

should be

protected function getConfig_FactoryService()
{
  $this->services['config.factory'] = $instance = new \Drupal\webprofiler\Config\ConfigFactoryWrapper(new \Drupal\Core\Config\ConfigFactory($this->get('config.storage'), $this->get('webprofiler.debug.event_dispatcher'), $this->get('config.typed')), $this->get('webprofiler.config'));

  $instance->_serviceId = 'config.factory';

  return $instance;
}

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because it prevents the use of a Symfony core feature
Issue priority Major because can expose developers to weird bugs (i.e. fatal errors if try to serialize a decorator)
Disruption Non-disruptive because Core doesn't use decorated services
CommentFileSizeAuthor
#1 2536370-1.patch1.85 KBlussoluca

Comments

lussoluca’s picture

StatusFileSize
new1.85 KB
lussoluca’s picture

Issue summary: View changes
lussoluca’s picture

Status: Active » Needs review
dawehner’s picture

Just to be clear, never expect it to be there. This is a pure implementation detail.

lussoluca’s picture

So a contrib cannot use decorated services? It seems a useful functionality.

dawehner’s picture

Do you mind checking whether decorated services work once #2531564: Fix leaky and brittle container serialization solution is applied?

lussoluca’s picture

Yes with the patch in https://www.drupal.org/node/2531564#comment-10133478 all my decorated services works.
Meanwhile I'm setting:

properties:
      _serviceId: 'service-name'

in webprofiler.services.yml for every decorated service as a workaround.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Let's ensure to add test coverage here.

cilefen’s picture

cilefen’s picture

Title: DependencySerializationTraitPass don't add _serviceId to decorated services » DependencySerializationTraitPass does not add _serviceId to decorated services
Issue summary: View changes
alexpott’s picture

Status: Needs work » Closed (duplicate)

Discussed with @effulgentsia, @catch, @xjm, @cilefen. We decided to mark this issue as a duplicate of #2531564: Fix leaky and brittle container serialization solution and require that that issue adds a test for this bug.