Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The service container is the central building block around the new dependency injection based architecture of Drupal 8. Under normal conditions, a cached version of the service container class is read form the disk early in the request. It is then instantiated exactly once. This instance is used throughout the request, such that services can be composed and reused as necessary. This is the common pattern implemented by all Symfony DI-based web applications and frameworks.
However, Drupal deviates from this pattern under very special circumstances. Namely the container definition can be updated and the container instance is then replaced by a new container whenever a module is installed / uninstalled. This also happens often when executing tests (except PHPUnit tests).
When the container is rebuilt in the middle of a request, there is a chance that services instantiated before the rebuild have obsolete state. Even worse, they may depend on other obsolete services.
Throughout the development of Drupal 8, there have been attempts to work around this problem by implementing two techniques:
- Instances of services marked with the
persisttag are restored after the container rebuild. Those services will automatically retain their state. However, as a consequence all dependencies of apersistservice also need to be declared to be persistent. - Recording on restoring state of some services is hard-coded into
DrupalKernel::initializeContainerand some other places
Both of those workarounds are cumbersome and error-prone. While it is necessary to maintain a limited amount of state across container rebuilds, it is also vitally important that code triggering container rebuilds is designed in a way, such that it does not retain any references to outdated services. This fact should be documented somewhere.
Comments
Comment #1
donquixote CreditAttribution: donquixote commentedI share your concerns, but personally would prefer to go further than what you suggest.
But yes, if we don't fix this, e.g. in the way that i suggest in #2282217, then at least this flaw / source of unpredictability needs to be documented.
Not necessarily. We could use the observer pattern to tell the persisted services about updated dependencies.
Not trivial and not happening in current D8 afaik, but it is an option.
(Disclaimer: I personally prefer a solution where we don't have to care about this at all)
The form submit handler or other component that enables a module is still from the universe of the old container. And it generally still exists, and has control. The same for the services or components that called the submit handler. The call stack will unwind, all through obsolete services.
Usually this does not matter much, because request handling for form submit usually don't change due to newly enabled modules.
But I would prefer if we could rely more on predictable than on incidental work flows.
Future contrib modules may want to call module install on the fly, and then continue to work with the new container version immediately.
This would be discouraged by the new documentation you suggest, but is this enough?
Also, think about hooks or events that may be added that fire after a form submit handler, or after a drush operation, while the call stack is still polluted with obsolete services.
But imo it would be safer to have something like #2282217: [meta] Never replace the container while services are still in call stack., where it is going to be technically impossible to do this.
The control flow / call stack should go back to something that is not owned by a container, before any container rebuild stuff can happen.
Comment #2
donquixote CreditAttribution: donquixote commentedBtw, how do we deal with services that become obsolete outside of a container rebuild?
E.g. if the language changes?
Comment #3
jhodgdonWhat exactly are you advocating needs to be documented? And where should it be documented? Is this issue actionable?
Comment #4
znerol CreditAttribution: znerol commentedComment #5
cilefen CreditAttribution: cilefen commentedComment #17
smustgrave CreditAttribution: smustgrave at Mobomo commentedSince there hasn't been any movement since moving to PNMI 8 years ago moving to outdated
If you feel it's still an issue please feel free to reopen with an updated issue summary