lievendoclo / Valkyrie-RCP

A fork from http://www.gitorious.org/valkyrie-rcp
Apache License 2.0
23 stars 10 forks source link

Closing with application.close() #3

Closed Nopalin closed 11 years ago

Nopalin commented 11 years ago

Hey, i dont want to fork the project just for this, i have made some changes but are personal ones, like remove all logging dependencies and just use commons-logging and log4j, but i dont think anyone are interested in that.

Ok the issue is with closing the app from close method in the DefaultApplication. Spring 3 has a new feature (well i dont know if it is new) where all beans by default are inferred by a dispose method called close (with no arguments). So spring register that dispose method in application bean, then when we call application.close(), it closes the windowManager, and then the context. Context is then marked as being closed but when it disposes the bean 'application' (because of the inferred method) close method is called again, fetching beans that does not exist, creating them but without injecting autowired properties, so a null pointer exception is raised.

The solution is simple, just marking the destroyMethof in the application bean declaration empty, like this:

@Bean(destroyMethod="") @Override public Application application() { return new DefaultApplication(); }

I did not know how to override that behavior, if in the overriden method i put the annotation @Bean again, spring register two beans with the same id, the last processed wins, but the last is the declared in the abstract class, so we do not resolve anything.

I do not check if other beans with close method in valkyrie have the same issues, it perhaps not, but i let you know if i found something.

Thanks for reading this and for my bad english.

ndeverge commented 11 years ago

Thanks for the issue and the explanation. It could be great if you could submit a pull request :-)

If you can't, I'll port your fix later.

cmadsen commented 11 years ago

I did a extends DefaultApplication and moved the onShutdown() to before the call to applicationContext()).close() in the close method:

@Override
public void close(boolean force, int exitCode) {
    forceShutdown1 = force;
    try {
        if (applicationConfig.windowManager().close()) {
                            // moved to before close
            applicationConfig.applicationLifecycleAdvisor().onShutdown();
            if (applicationConfig.applicationContext() instanceof ConfigurableApplicationContext) {
                ((ConfigurableApplicationContext) applicationConfig
                        .applicationContext()).close();
            }
            // applicationConfig.applicationLifecycleAdvisor().onShutdown();
        }
    } finally {
        if (force) {
            System.exit(exitCode);
        }
    }
}
Nopalin commented 11 years ago

That does not resolve the bug, the problem is the close method of application bean registered by the container as a disposable method. So no matter if you put lifecicleAdvisor.close() before, in the moment you call appContext.close(), it will call again the close method on valkyrie application bean, calling again windowManager.close(), and lifeycicleAdvisor.onShutDown() and again context.close(). This is the real problem because those beans does not exist anymore, letting springto create them again, with no injecting autowired props and raised an exception.

The solution is to avoid the container to register close method if application bean as a disposable, but telling it that, and calling manually when requeired, like exit command.

Greetings