+1

@fabpot could this be merged ? The regression in 2.3 for resetManager is quite annoying.

+1

I have tested this PR as is and it still doesn't fix the problem being addressed.
There is an array_key_exists check

// re-use shared service instance if it exists
if (array_key_exists($id, $this->services)) {
    return $this->services[$id];
}

which makes the service container return null values. However, changing that to isset() raises another issues, I started to get the following error: You have requested a synthetic service ("request"). The DIC does not know how to construct this service.

@stloyd @fabpot any news on this?

+1, this unfortunately limits us upgrading from 2.2.x to 2.3.x

+1

This fix will not work completely.

    public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE)
    {
        $id = strtolower($id);

        // resolve aliases
        if (isset($this->aliases[$id])) {
            $id = $this->aliases[$id];
        }

        // re-use shared service instance if it exists
        if (array_key_exists($id, $this->services)) {
            return $this->services[$id];
        }

You missed this part in your pull request. This array_key_exists has to be changed to isset() as well.

This PR cannot be merged as the change from isset to array_key_exists was done for a reason. So, reverting everything back to isset is not an option. We need to investigate the issue further to find another solution.

@fabpot The change from isset to array_key_exists is also what broke the registry in 2.3 (and it was not covered by tests).

If setting the service to null does not allow resetting it anymore, I don't think we can implement the resetting without adding another method. It would mean a second BC rbeak to mitigate the effect of the first one.

random brain spin

May I suggest placeholders to indicate what is supposed to happen with the service key?

    protected function resetService($name)
    {
        $this->container->set($name, new ServiceReset('or how ever you want to call it'));
    }

This could tell the get() to return it but re-initialize the initial service before the actual return happens.

@fabpot what was the reason for that particular change? Or do you have a reference as to where this was discussed? Only thing I can seem to find is the commit (ec1e7ca), but that description does not make things much clearer to me.

Anyway, as it seems to me it does not make sense to have entries in the services array with value null. So why not check in the set method if the given value is null, and if so, unsetting the particular entry if it exists? This would fix the BC break AFAIK without having to add another method.

@jankramer isset has been replaced with array_key_exists in PR #7007 which introduces synchronized services.

Can you test PR #8582 which should fix the issue and keep BC?

Symfony tests pass and my local tests tell me that it fixes the issue with Doctrine as well. So, just waiting for some more confirmation from more real-world projects before merging the fix.

Yep, just tested and this fixes some issues for me as well.

I've just upgraded one of my projects that had this issue and my patch does fix the problem.

Closing in favor of #8582