If I understand correctly, it can currently happen that an event causes the container to be rebuilt and replaced in the middle of a request.
Services that originate from the old container may still be in use somewhere, whereas others are replaced.
And then there are the "persisted services"..
Somehow this all seems to work, but it is scary to think about.
I do not claim to perfectly understand all of it, not even close.
But I bet that a lot of things would be easier if the container would not change in the middle of something, but only at specific points in the request where it is "safe".
Speaking in a metaphor:
Don't tear down + rebuild a house from the inside. Do it from the outside.
And if you are still inside and think that you want to rebuild it, take notes instead. Draft a plan of the new house, then leave the old one, and then tear it down and rebuild it following the draft.
In Drupal, this would mean:
- Create a container instance, and provide it with a "queue object".
- Use the container services to handle the request.
Whenever a component (e.g. a form callback) wants to install a module or change configuration, queue it up for later. - Shut down the container, forget all its services.
- Execute the queued operations, outside of any container.
- If any of the operations would trigger events that need container services, e.g. hook_install() stuff, create a new container instance to do this.
This would all go nicely together with #2272129: [meta] Introduce a low-level DIC to organize low-level services, but it may also work standalone.
Comments
Comment #1
donquixote commentedComment #2
donquixote commentedBtw, this would also allow us to stop worrying about the $allow_dumping parameter.
Comment #3
donquixote commentedComment #4
znerol commentedI agree that rebuilding the container mid-request is very adventurous (though extremely rare under normal circumstances) and I also agree that we should get rid of
persistservices completely. At this point in the release cycle I'd rather prefer to introduce lower-risk changes like #2281903: Refactor DrupalKernel::initializeContainer in preparation of a replacement for persist services and #2282371: Document the fact that it is not safe to reuse services across container rebuilds instead.Comment #5
donquixote commentedNot really.
The call stack is still full of obsolete services, when e.g. a form submit handler enables a module.
Usually these don't do much harmful, but I would certainly say it's a source of unpredictability. And we already have too much of that in Drupal.
Comment #6
znerol commentednormal circumstances == production environment
Comment #7
donquixote commentedI personally don't care that much..
The bootstrap, module install, container management, all that is currently a mess of unpredictability, full of stuff that people are afraid to touch. We continuously lose time working around these problems, and it will follow us around during the D8 life cycle. It won't become easier with contrib in the mix.
Some cleanup needs to happen before a Drupal 8 stable, and I would prefer a solid solution with a real plan.
Module install does happen on production too..
Sure, not on the average visitor request. But weird stuff happening on module install can have side effects beyond that request.
I agree it might be rare that we really see issues because of this. It is just that I have grown a sensibility to anything that lacks or hurts predictability.
Btw I don't see you on IRC..
Comment #8
donquixote commentedRelated: #2282779: [meta] DrupalKernel has too many responsibilities
And the patches proposed make it even bigger..
Comment #9
donquixote commentedSee also http://stackoverflow.com/questions/24182990/dependency-injection-contain...
Comment #23
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #24
smustgrave commentedSince it's been 3+ months without a follow up going to close out. If still a valid issue please please re-open!
Thanks all!
Comment #25
donquixote commentedI think this is a dead end issue :)
There are always services in the call stack, and so far this seems to be ok.
The services from the old container create a new container, and that's how it works.