Cannot load more than 1 dependency module from init() #5194

Closed
hschletz opened this Issue Sep 30, 2013 · 10 comments

Projects

None yet

4 participants

@hschletz

I have an application with several modules which are designed to take care of loading other modules they depend on, like this:

class Master {
    public function init($manager)
    {
        $manager->LoadModule('Dep1');
        $manager->LoadModule('Dep2');
    }
    public function onBootstrap($e)
    {
        print "<p>Bootstrap Master</p>\n";
    }
}

class Dep1 {
    public function onBootstrap($e)
    {
        print "<p>Bootstrap Dep1</p>\n";
    }
}

class Dep2 {
    public function onBootstrap($e)
    {
        print "<p>Bootstrap Dep2</p>\n";
    }
}

The initialization code loads the "Master" module, which loads its dependencies Dep1 and Dep2. The expected output would be:

Bootstrap Dep1
Bootstrap Dep2
Bootstrap Master

Instead I get:

Bootstrap Dep1
Bootstrap Dep2
Bootstrap Dep2

onBootstrap() gets called twice for Dep2, and never for Master. This happens to all listeners that get invoked after InitTrigger. By default, these are OnBootstrapListener, LocatorRegistrationListener, ConfigListener and ServiceListener. It doesn't matter which module actually implements the corresponding method - the problem comes from the listeners fed with an incorrectly set up event.

Everything works as expected if only 1 module is loaded from init(). If I disable the line

$this->loadFinished = true;

in loadModule(), the example works as expected, but I don't really understand the module loading logic and I doubt that's the correct fix. I think it's an error to trace module loading status in a single variable, as there are interferences between modules. Maybe $loadFinished should be made an associative array, to separate loading status per module.

@ClemensSahs

I don't see the benefits of this var? In my opinion we need in every case a cloned event object, right?

Here is the commit from
88eed43

I have tow possible solutions:

// Zend\ModuleManager\Listener\InitTrigger.php

    /**
     * @param ModuleEvent $e
     * @return void
     */
    public function __invoke(ModuleEvent $e)
    {
        $module = $e->getModule();
        if (!$module instanceof InitProviderInterface
            && !method_exists($module, 'init')
        ) {
            return;
        }
        $moduleName = $e->getModuleName();
        $module->init($e->getTarget());

        if ( $e->getModule() !== $module ) {
            $e->setModule($module);
            $e->setModule($moduleName);
        }
    }

OR

Zend\ModuleManager\ModuleManager.php

clone the event in the methode loadModule

ON both we have the right result with predefined modules and late defined modules

@EvanDotPro EvanDotPro was assigned Oct 18, 2013
@hschletz

Although my bug description covers only InitTrigger (as this would be the most likely place to load dependencies), calling loadModule() from any listener would suffer from the same problem, affecting any other listeners that are triggered afterwards. For that reason, curing symptoms in InitTrigger would work for this particular use case, but leave the problem elsewhere.

My workaround (commenting out a line) implicitly causes the event to be cloned unconditionally, as you suggested. If there is something wrong with that approach and tracking the loading status is required, it must be done per module, not in a single boolean shared between all loadModule() invocations.

@ClemensSahs

@hschletz
my favorite solutions is the clone of ModuleEvent, too. I see no reason why no.

@EvanDotPro

is there some problem we not see? If we clone the event object by default in the method ModuleManager::loadModule()

@ClemensSahs

so I'm write a little test but I can't something is wrong with it, in a localcase ( with ZendSkeletonApplication ) the onBootstrap event will not called but in the "test" it will be true...
Have anybody a Idee?
here my branch for the this test case

@weierophinney
Zend Framework member

@EvanDotPro ping...

@ClemensSahs

Looks like @EvanDotPro is busy... has somebody else a idea what I have miss in this test

in the Zend SkeletonApplication we have this issued but I can't rebuild as test

thanks for help

@hschletz

I checked out your branch and successfully used it for 2 different applications - both init() and onBootstrap() get called. The branch looks OK - there seems to be a problem with your application or your local configuration.

@ClemensSahs

@hschletz
no I only have this issues in the skeletonApplication with out me patch... with me patch i fix this bug

But this behavior I cant rebuild in a tests, so i can not say the test is valid this issues correctly.

this is the reason I ask evan for a little idea

@hschletz

Again, since I can't reproduce the problem with my own applications, I suspect a problem with your test application/configuration. If you provide me the code of your application, I could investigate.

@hschletz

The issue has been fixed via #5651.

@hschletz hschletz closed this Apr 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment