-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Bug with sharing of non-public service #23979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
[edit] see the next comment, the issue exists. #23979 (comment) Here is a reproducer that show there is no problem. I used Symfony 3.3.6 to make my test. $ composer show
symfony/symfony v3.3.6 The Symfony PHP framework <?php
namespace GlobalNamespace\Foobar {
class NewLine {
public function __construct()
{
echo "I'm new line and i'm used everywhere";
$this->hello();
}
public function hello()
{
echo "\n";
}
}
}
namespace GlobalNamespace\Foo {
class Stof {
private $nl;
public function __construct(\GlobalNamespace\Foobar\Newline $newline)
{
$this->nl = $newline;
}
public function hello()
{
echo "You missed something line 150, please change it !";
$this->nl->hello();
}
}
}
namespace GlobalNamespace\Bar {
class Fabpot {
private $nl;
public function __construct(\GlobalNamespace\Foobar\Newline $newline)
{
$this->nl = $newline;
}
public function hello()
{
echo "[MERGED] Thanks for your contribution !";
$this->nl->hello();
}
}
}
namespace GlobalNamespace {
require 'vendor/autoload.php';
use GlobalNamespace\Bar\Fabpot;
use GlobalNamespace\Foo\Stof;
use GlobalNamespace\Foobar\NewLine;
use Symfony\Component\DependencyInjection\ContainerBuilder;
$container = new ContainerBuilder();
$container->register('nl', NewLine::class)->setPublic(false);
$container->autowire('stof', Stof::class)->setPublic(true);
$container->autowire('fabpot', Fabpot::class)->setPublic(true);
$container->compile();
$container->get('stof')->hello();
$container->get('fabpot')->hello();
} Output:
(expected: only one call to "new line constructor"... And it is !) Status: Reviewed |
I found a bug anyway. Let's take the previous code with the following usage: $container = new ContainerBuilder();
$container->register('nl', NewLine::class)->setPublic(false)->setShared(false);
$container->autowire('stof', Stof::class)->setPublic(true);
$container->autowire('fabpot', Fabpot::class)->setPublic(true);
$container->compile();
$container->get('stof')->hello();
$container->get('fabpot')->hello(); Output:
Also, the spl hash is the same. Of course. But I asked that the instance is not shared. Here is the issue. If I set it public the issue is the same, but it works when I call it from the outside: $container = new ContainerBuilder();
$container->register('nl', NewLine::class)->setPublic(true)->setShared(false);
$container->autowire('stof', Stof::class)->setPublic(true);
$container->autowire('fabpot', Fabpot::class)->setPublic(true);
$container->compile();
$container->get('stof')->hello(); // instanciates "NewLine"
$container->get('fabpot')->hello(); // reuse the existing instance
$container->get('nl')->hello(); // instanciates "NewLine" again |
Cannot reproduce it with an "artificial" test, works fine, but with real project there is still an issue. I going to continue to work with it. |
I did it for you @mikemeier :). See my last post. The issue is real. |
@Nek- isnt that autowiring kicking in? Creating a new I'd say no issue then :) |
@ro0NL the autowiring doesn't instantiate many times the service as expected. (As I understand you describe the inverse case, which would make eventually sense but would still remain an issue) The case here in a nutshell:
The last bunch of code I posted exposes this issue. |
@Nek- your autowiring issue is that your non-shared service is not registered for the class name as id. So it is not the one used by the autowiring (actually, it may be the one being used due to the BC layer, as you use 3.3 and not master. We may have a weird thing in the BC layer). The autowiring may be creating a new service for Newline (and services created there are shared). |
@Nek- can you dump |
Here is the output: $ php test.php
[New instance NewLine] I'm new line and i'm used everywhere
You missed something line 150, please change it !
[MERGED] Thanks for your contribution !
[New instance NewLine] I'm new line and i'm used everywhere
array(3) {
[0]=>
string(143) "Symfony\Component\DependencyInjection\Compiler\AutowirePass: Type "GlobalNamespace\Foobar\Newline" has been auto-registered for service "stof"."
[1]=>
string(147) "Symfony\Component\DependencyInjection\Compiler\RemovePrivateAliasesPass: Removed service "Psr\Container\ContainerInterface"; reason: private alias."
[2]=>
string(171) "Symfony\Component\DependencyInjection\Compiler\RemovePrivateAliasesPass: Removed service "Symfony\Component\DependencyInjection\ContainerInterface"; reason: private alias."
} Click me to see the code that generates this output<?php
namespace GlobalNamespace\Foobar {
class NewLine {
public function __construct()
{
echo "[New instance NewLine] I'm new line and i'm used everywhere";
$this->hello();
}
public function hello()
{
echo "\n";
}
}
}
namespace GlobalNamespace\Foo {
class Stof {
private $nl;
public function __construct(\GlobalNamespace\Foobar\Newline $newline)
{
$this->nl = $newline;
}
public function hello()
{
echo "You missed something line 150, please change it !";
$this->nl->hello();
}
}
}
namespace GlobalNamespace\Bar {
class Fabpot {
private $nl;
public function __construct(\GlobalNamespace\Foobar\Newline $newline)
{
$this->nl = $newline;
}
public function hello()
{
echo "[MERGED] Thanks for your contribution !";
$this->nl->hello();
}
}
}
namespace GlobalNamespace {
require 'vendor/autoload.php';
use GlobalNamespace\Bar\Fabpot;
use GlobalNamespace\Foo\Stof;
use GlobalNamespace\Foobar\NewLine;
use Symfony\Component\DependencyInjection\ContainerBuilder;
$container = new ContainerBuilder();
$container->register('nl', NewLine::class)->setPublic(true)->setShared(false);
$container->autowire('stof', Stof::class)->setPublic(true);
$container->autowire('fabpot', Fabpot::class)->setPublic(true);
$container->compile();
$container->get('stof')->hello();
$container->get('fabpot')->hello();
$container->get('nl')->hello();
var_dump($container->getCompiler()->getLog());
} |
#24671 to the rescue? Can you try setting the parameter? And upgrade to beta2 of course. |
Closing as this should be fixed, as suggested. Please report back if not. |
If service declared as non-public it stops to be shared, means for every consumer of such a service a new one will be instantiated. Explicit
shared: true
makes no difference. Withpublic: true
works as expected (shared by default).Behaviour has been noticed for compiled ContainerBuilder, but not dumped.
The text was updated successfully, but these errors were encountered: