On 3/8/2011 6:31 AM, Simon Laws wrote:
I checked in a change under TUSCANY-3842 at revision: 1079366 which deserves a bit of discussion. This was motivated by playing with a node restart test that Ant added [1]. Looking at the code in NodeImpl.start() it used to do the following:try { if (contributions == null) { contributions = nodeFactory.loadContributions(configuration, context); } domainComposite = nodeFactory.configureNode(configuration, contributions, context); this.compositeContext = new CompositeContext(nodeFactory.registry, endpointRegistry, domainComposite, configuration.getDomainURI(), configuration.getURI(), nodeFactory.getDeployer().getSystemDefinitions()); } finally { // Reset the thread context monitor nodeFactory.monitorFactory.setContextMonitor(tcm); } // Activate the composite compositeActivator.activate(compositeContext, domainComposite); // Start the composite compositeActivator.start(compositeContext, domainComposite); This takes a long time when a node is restarted and it strikes me as strange that the domainComposite is recreated each time even when contributions are not reloaded. I've changed this in the new revision so that the domainComposite and compositeContext are only created the first time through when the contributions are loaded. I.e. if (contributions == null) { contributions = nodeFactory.loadContributions(configuration, context); domainComposite = nodeFactory.configureNode(configuration, contributions, context); this.compositeContext = new CompositeContext(nodeFactory.registry, endpointRegistry, domainComposite, configuration.getDomainURI(), configuration.getURI(), nodeFactory.getDeployer().getSystemDefinitions()); } This gets a clean build on Windows (and rebuilding in Linux now) but it seems quite a fundamental change so I'm raising it here. I think we originally intended nodes to follow a configure once and start and stop as many times model. I think this fits in with the code the way it is now. However the code may have ended up the way it did for good reason. If it did I don't remember now what the reason was. As an aside we do need to have a conversation about what happens when configuration changes in a domain which affects whether a running node or a node that has been configured but stopped. That's for another thread though. [1] http://svn.apache.org/repos/asf/tuscany/sca-java-2.x/trunk/modules/node-impl/src/test/java/org/apache/tuscany/sca/node/impl/PerflTest.java Regards Simon
I believe this change breaks NodeFactoryImpl.createNode(List<?>), because it uses the NodeImpl constructor that initializes the contributions instance variable to an empty list. When NodeImpl.start() is called, an NPE is thrown, because the domainComposite instance variable does not get initialized.
