Decorated services crash on serialization.

#16 is working for me, running Drupal 8.8.6; before the patch, a decorated 'flood' service was being serialized this way (separate of the _serviceIds array for its nesting level):

...{s:8:" * flood";O:22:"Drupal\my_module\MyFlood":2:{s:14:" * _serviceIds";a:3:{s:12:"innerService";s:5:"flood";s:10:"connection";s:8:"database";s:12:"requestStack";s:13:"request_stack";}...

After I applied the patch, the decorated flood service is now part of the "_serviceIds" array:

...s:14:" * _serviceIds";a:10:{s:14:"entity_manager";s:14:"entity.manager";s:6:"client";s:16:"my_module.api_client";s:15:"my_module_user_manager";s:18:"my_module.user_manager";s:15:"email_validator";s:15:"email.validator";s:5:"flood";s:5:"flood";s:8:"renderer";s:8:"renderer";s:15:"someService";s:20:"some.service";s:12:"requestStack";s:13:"request_stack";s:13:"configFactory";s:14:"config.factory";s:17:"stringTranslation";s:18:"string_translation";}...

Also, using drush php:cli shows proper behavior now:

// Without #16
>>> $flood = \Drupal::service('flood');
=> Drupal\my_module\MyFlood {#9064}

// With #16
>>> $flood = \Drupal::service('flood');
=> Drupal\my_module\MyFlood {#11317
     +"_serviceId": "flood",
   }

// Temporarily exposed $innerService to peek into it.
>>> $flood->innerService;                                                                                                                                                                                                                                                                                            => Drupal\Core\Flood\DatabaseBackend {#9880
     +"_serviceId": "my_module.my_flood.inner",
   }

This is the decorated service definition:

  my_module.my_flood:
    class: Drupal\my_module\MyFlood
    public: false
    decorates: flood
    arguments: ['@my_module.my_flood.inner', '@database', '@request_stack']

Thanks a lot @cburschka! :)

It looks ready

@cburschka well past you said

This definitely needs a kernel test case, especially with multiple decorators on the same service.

And the current test does not have that so... back to needs work. It might be good to push on #2531564: Fix leaky and brittle container serialization solution as the better fix.

It's true that the other issue will make this fix obsolete when it's done, but it's not yet clear if it'll be possible to push it to D8 or if it will end up requiring deprecations, so this smaller bugfix might still be worthwhile. :)

That's a good point.

Thanks for improving the test.

It could be affected by https://github.com/symfony/symfony/pull/41407 which is included into latest 4.4.25 release

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/DependencySerializationTraitPass.php
@@ -16,12 +16,34 @@ class DependencySerializationTraitPass implements CompilerPassInterface {
+      if (!$renamedId) {
+        $renamedId = $service_id . '.inner';

hardcoding the name is fragile approach

@andypost, re #29, isn't it what upstream also does? Or what is your expected solution here?

hardcoding the name is fragile approach

https://github.com/symfony/symfony/blob/eda7eb3cd8876e761fde46d65b0658a4...

After a year without a progress, I think it needs a reroll against 9.5.x.

Cool, let's hide "outdated" files from the issue description because the MR+the test only patch that get updated from now.

Test added and fix looks great

Hopefully #2531564: Fix leaky and brittle container serialization solution will make some progress someday. As it has not for now lets get this done...

Committed and pushed b16df24b08 to 10.1.x and 4c126fd390 to 10.0.x and c0cdaa7151 to 9.5.x and 1704f1191d to 9.4.x. Thanks!