confirming, unbinding before cdi destruction prevent to use cdi/EE integration properly, will revert
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://blog-rmannibucau.rhcloud.com> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory <https://javaeefactory-rmannibucau.rhcloud.com> 2017-03-03 21:59 GMT+01:00 Romain Manni-Bucau <[email protected]>: > Null checks should be reverted please as said on the list. > > About the critical part of the diff can you say a word here why it is > needed? War and ear behave differently and i fear now we can break some > working undeployments so trying to avoid to break it for the 7.0.3 - we > were quite stable and close to release. > > ---------- Message transféré ---------- > De : <[email protected]> > Date : 3 mars 2017 21:25 > Objet : [3/3] tomee git commit: Check for null and clean WebBeansContext > later > À : <[email protected]> > Cc : > > Check for null and clean WebBeansContext later >> >> >> Project: http://git-wip-us.apache.org/repos/asf/tomee/repo >> Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/9b7b4657 >> Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/9b7b4657 >> Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/9b7b4657 >> >> Branch: refs/heads/master >> Commit: 9b7b46570cf43afa9bb904e1615794400124539a >> Parents: de09ee0 >> Author: AndyGee <[email protected]> >> Authored: Fri Mar 3 21:24:42 2017 +0100 >> Committer: AndyGee <[email protected]> >> Committed: Fri Mar 3 21:24:42 2017 +0100 >> >> ---------------------------------------------------------------------- >> .../openejb/assembler/classic/Assembler.java | 166 >> ++++++++++--------- >> 1 file changed, 85 insertions(+), 81 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/tomee/blob/9b7b4657/c >> ontainer/openejb-core/src/main/java/org/apache/openejb/assem >> bler/classic/Assembler.java >> ---------------------------------------------------------------------- >> diff --git >> a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java >> b/container/openejb-core/src/main/java/org/apache/openejb/as >> sembler/classic/Assembler.java >> index cdf7ed2..9af8153 100644 >> --- a/container/openejb-core/src/main/java/org/apache/openejb/as >> sembler/classic/Assembler.java >> +++ b/container/openejb-core/src/main/java/org/apache/openejb/as >> sembler/classic/Assembler.java >> @@ -377,8 +377,8 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Object filter = loader.loadClass("org.apache.o >> penejb.bval.BValCdiFilter").newInstance(); >> loader.loadClass("org.apache.bval.cdi.BValExtension") >> .getMethod( >> - "setAnnotatedTypeFilter", >> - loader.loadClass("org.apache.b >> val.cdi.BValExtension$AnnotatedTypeFilter")) >> + "setAnnotatedTypeFilter", >> + loader.loadClass("org.apache.b >> val.cdi.BValExtension$AnnotatedTypeFilter")) >> .invoke(null, filter); >> } catch (final Throwable th) { >> // ignore, bval not compatible or not present >> @@ -579,7 +579,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final ContainerSystem component = systemInstance.getComponent(Co >> ntainerSystem.class); >> if (component != null) { >> postConstructResources(rIds, >> ParentClassLoaderFinder.Helper.get(), >> component.getJNDIContext(), null); >> - }else{ >> + } else { >> throw new RuntimeException("ContainerSystem has not been >> initialzed"); >> } >> >> @@ -1320,8 +1320,8 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> >> private static List<CommonInfoObject> >> listCommonInfoObjectsForAppInfo(final >> AppInfo appInfo) { >> final List<CommonInfoObject> vfs = new >> ArrayList<CommonInfoObject>( >> - appInfo.clients.size() + appInfo.connectors.size() + >> - appInfo.ejbJars.size() + appInfo.webApps.size()); >> + appInfo.clients.size() + appInfo.connectors.size() + >> + appInfo.ejbJars.size() + appInfo.webApps.size()); >> for (final ClientInfo clientInfo : appInfo.clients) { >> vfs.add(clientInfo); >> } >> @@ -1468,12 +1468,12 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final MethodContext methodContext = >> entry.getValue(); >> for (final ScheduleData scheduleData : >> methodContext.getSchedules()) { >> timerStore.createCalendarTime >> r(timerService, >> - (String) >> beanContext.getDeploymentID(), >> - null, >> - entry.getKey(), >> - scheduleData.getExpression(), >> - scheduleData.getConfig(), >> - true); >> + (String) >> beanContext.getDeploymentID(), >> + null, >> + entry.getKey(), >> + scheduleData.getExpression(), >> + scheduleData.getConfig(), >> + true); >> } >> } >> beanContext.setEjbTimerService(timerService); >> @@ -1501,11 +1501,11 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> } >> >> beanContext.set( >> - BeanContext.ProxyClass.class, >> - new BeanContext.ProxyClass( >> - beanContext, >> - interfaces.toArray(new >> Class<?>[interfaces.size()]) >> - )); >> + BeanContext.ProxyClass.class, >> + new BeanContext.ProxyClass( >> + beanContext, >> + interfaces.toArray(new >> Class<?>[interfaces.size()]) >> + )); >> } >> } >> // process application exceptions >> @@ -1583,7 +1583,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> if >> (container.getBeanContext(deployment.getDeploymentID()) >> == null) { >> container.deploy(deployment); >> if (!((String) deployment.getDeploymentID()). >> endsWith(".Comp") >> - && !deployment.isHidden()) { >> + && !deployment.isHidden()) { >> logger.info("createApplication.createdEjb", >> deployment.getDeploymentID(), deployment.getEjbName(), >> container.getContainerID()); >> } >> if (logger.isDebugEnabled()) { >> @@ -1604,7 +1604,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Container container = >> deployment.getContainer(); >> container.start(deployment); >> if (!((String) deployment.getDeploymentID()). >> endsWith(".Comp") >> - && !deployment.isHidden()) { >> + && !deployment.isHidden()) { >> logger.info("createApplication.startedEjb", >> deployment.getDeploymentID(), deployment.getEjbName(), >> container.getContainerID()); >> } >> } catch (final Throwable t) { >> @@ -1659,10 +1659,10 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> try { >> final MBean annotation = clazz.getAnnotation(MBean.clas >> s); >> final ObjectName leaf = annotation == null || >> annotation.objectName().isEmpty() ? new ObjectNameBuilder("openejb.use >> r.mbeans") >> - .set("application", id) >> - .set("group", clazz.getPackage().getName()) >> - .set("name", clazz.getSimpleName()) >> - .build() : new ObjectName(annotation.objectName()); >> + .set("application", id) >> + .set("group", clazz.getPackage().getName()) >> + .set("name", clazz.getSimpleName()) >> + .build() : new ObjectName(annotation.objectNa >> me()); >> >> server.registerMBean(new DynamicMBeanWrapper(wc, >> instance), leaf); >> appMbeans.put(mbeanClass, leaf.getCanonicalName()); >> @@ -1680,8 +1680,8 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> WebBeansContext webBeansContext = appContext.get(WebBeansContext >> .class); >> if (webBeansContext == null) { >> webBeansContext = appContext.getWebBeansContext(); >> - }else{ >> - if (null == appContext.getWebBeansContext()){ >> + } else { >> + if (null == appContext.getWebBeansContext()) { >> appContext.setWebBeansContext(webBeansContext); >> } >> return; >> @@ -1926,7 +1926,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> Object object; >> try { >> object = LazyResource.class.isInstance(inObject) && >> LazyResource.class.cast(inObject).isInitialized() ? >> - LazyResource.class.cast(inObject).getObject() : >> inObject; >> + LazyResource.class.cast(inObject).getObject() : >> inObject; >> } catch (final NamingException e) { >> object = inObject; // in case it impl DestroyableResource >> } >> @@ -2039,7 +2039,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Iterator<ResourceInfo> iterator; >> if (configuration != null) { >> iterator = configuration.facilities.resou >> rces.iterator(); >> - }else{ >> + } else { >> throw new Exception("OpenEjbConfiguration has not been >> initialized"); >> } >> while (iterator.hasNext()) { >> @@ -2110,48 +2110,37 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> deployedApplications.remove(appInfo.path); >> logger.info("destroyApplication.start", appInfo.path); >> >> - final Context globalContext = containerSystem.getJNDIContext >> (); >> final AppContext appContext = containerSystem.getAppContext( >> appInfo.appId); >> - final ClassLoader classLoader = appContext.getClassLoader(); >> - >> - SystemInstance.get().fireEvent(new >> AssemblerBeforeApplicationDestroyed(appInfo, appContext)); >> >> //noinspection ConstantConditions >> if (null == appContext) { >> logger.warning("Application id '" + appInfo.appId + "' >> not found in: " + Arrays.toString(containerSystem.getAppContextKeys())); >> return; >> - } else { >> - final WebBeansContext webBeansContext = >> appContext.getWebBeansContext(); >> - if (webBeansContext != null) { >> - final ClassLoader old = >> Thread.currentThread().getContextClassLoader(); >> - Thread.currentThread().setCont >> extClassLoader(classLoader); >> - try { >> - final ServletContext context = >> appContext.isStandaloneModule() && >> appContext.getWebContexts().iterator().hasNext() >> ? >> - appContext.getWebContexts().it >> erator().next().getServletContext() : null; >> - webBeansContext.getService(Con >> tainerLifecycle.class).stopApplication(context); >> - } finally { >> - Thread.currentThread().setCont >> extClassLoader(old); >> - } >> - } >> - final Map<String, Object> cb = appContext.getBindings(); >> - for (final Entry<String, Object> value : cb.entrySet()) { >> - String path = value.getKey(); >> - if (path.startsWith("global")) { >> - path = "java:" + path; >> - } >> - if (!path.startsWith("java:global")) { >> - continue; >> - } >> + } >> + >> + SystemInstance.get().fireEvent(new >> AssemblerBeforeApplicationDestroyed(appInfo, appContext)); >> + >> + final Context globalContext = containerSystem.getJNDIContext >> (); >> >> - unbind(globalContext, path); >> - unbind(globalContext, "openejb/global/" + >> path.substring("java:".length())); >> - unbind(globalContext, path.substring("java:global".l >> ength())); >> + final Map<String, Object> cb = appContext.getBindings(); >> + for (final Entry<String, Object> value : cb.entrySet()) { >> + String path = value.getKey(); >> + if (path.startsWith("global")) { >> + path = "java:" + path; >> } >> - if (appInfo.appId != null && !appInfo.appId.isEmpty() && >> !"openejb".equals(appInfo.appId)) { >> - unbind(globalContext, "global/" + appInfo.appId); >> - unbind(globalContext, appInfo.appId); >> + if (!path.startsWith("java:global")) { >> + continue; >> } >> + >> + unbind(globalContext, path); >> + unbind(globalContext, "openejb/global/" + >> path.substring("java:".length())); >> + unbind(globalContext, path.substring("java:global".l >> ength())); >> } >> + if (appInfo.appId != null && !appInfo.appId.isEmpty() && >> !"openejb".equals(appInfo.appId)) { >> + unbind(globalContext, "global/" + appInfo.appId); >> + unbind(globalContext, appInfo.appId); >> + } >> + >> >> final EjbResolver globalResolver = new EjbResolver(null, >> EjbResolver.Scope.GLOBAL); >> for (final AppInfo info : deployedApplications.values()) { >> @@ -2245,6 +2234,8 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> for (final WebContext webContext : >> appContext.getWebContexts()) { >> containerSystem.removeWebContext(webContext); >> } >> + >> + final ClassLoader classLoader = appContext.getClassLoader(); >> TldScanner.forceCompleteClean(classLoader); >> >> // Clear out naming for all components first >> @@ -2294,7 +2285,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> } >> } catch (final NamingException e) { >> undeployException.getCauses().add(new Exception("Unable >> to prune openejb/Deployments and openejb/local namespaces, this could cause >> future deployments to fail.", >> - e)); >> + e)); >> } >> >> deployments.clear(); >> @@ -2412,6 +2403,19 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> } >> } >> >> + final WebBeansContext webBeansContext = >> appContext.getWebBeansContext(); >> + if (webBeansContext != null) { >> + final ClassLoader old = Thread.currentThread().getCont >> extClassLoader(); >> + Thread.currentThread().setCont >> extClassLoader(classLoader); >> + try { >> + final ServletContext context = >> appContext.isStandaloneModule() && >> appContext.getWebContexts().iterator().hasNext() >> ? >> + appContext.getWebContexts().it >> erator().next().getServletContext() : null; >> + webBeansContext.getService(Con >> tainerLifecycle.class).stopApplication(context); >> + } finally { >> + Thread.currentThread().setContextClassLoader(old); >> + } >> + } >> + >> for (final String id : appInfo.containerIds) { >> removeContainer(id); >> } >> @@ -2461,7 +2465,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> } >> >> final LazyObjectReference<?> ref = >> LazyObjectReference.class.cast(binding.getObject()); >> - if (! ref.isInitialized()) { >> + if (!ref.isInitialized()) { >> globalContext.unbind(name); >> removeResourceInfo(name); >> return; >> @@ -2529,7 +2533,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final ClassLoaderEnricher component = >> SystemInstance.get().getComponent(ClassLoaderEnricher.class); >> if (component != null) { >> jars.addAll(Arrays.asList(component.applicationEnrichment() >> )); >> - }else { >> + } else { >> logger.warning("Unable to find open-jpa-integration jar"); >> } >> >> @@ -2578,7 +2582,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> allIsIntheClasspath = false; >> if (logger.isDebugEnabled()) { >> logger.debug(url.toExternalForm() + " (" + >> URLs.toFile(url) >> - + ") is not in the classloader so we'll >> create a dedicated classloader for this app"); >> + + ") is not in the classloader so >> we'll create a dedicated classloader for this app"); >> } >> break; >> } >> @@ -2622,7 +2626,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> initialContext = new InitialContext(contextInfo.properties); >> } catch (final NamingException ne) { >> throw new >> OpenEJBException(String.format("JndiProvider(id=\"%s\") >> could not be created. Failed to create the InitialContext using the >> supplied properties", >> - contextInfo.id), ne); >> + contextInfo.id), ne); >> } >> >> try { >> @@ -2801,7 +2805,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> public void createResource(final Collection<ServiceInfo> infos, >> final ResourceInfo serviceInfo) throws OpenEJBException { >> final boolean usesCdiPwdCipher = usesCdiPwdCipher(serviceInfo); >> final Object service = "true".equalsIgnoreCase(String >> .valueOf(serviceInfo.properties.remove("Lazy"))) || usesCdiPwdCipher ? >> - newLazyResource(infos, serviceInfo) : >> + newLazyResource(infos, serviceInfo) : >> doCreateResource(infos, serviceInfo); >> if (usesCdiPwdCipher && !serviceInfo.properties.contai >> ns("InitializeAfterDeployment")) { >> serviceInfo.properties.put("InitializeAfterDeployment", >> "true"); >> @@ -2812,7 +2816,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> bindResource(alias, service, false); >> } >> if (serviceInfo.originAppName != null && >> !serviceInfo.originAppName.isEmpty() && !"/".equals(serviceInfo.origin >> AppName) >> - && !serviceInfo.id.startsWith("global")) { >> + && !serviceInfo.id.startsWith("global")) { >> final String baseJndiName = >> serviceInfo.id.substring(serviceInfo.originAppName.length() >> + 1); >> serviceInfo.aliases.add(baseJndiName); >> final ContextualJndiReference ref = new >> ContextualJndiReference(baseJndiName); >> @@ -2924,16 +2928,16 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Object encoding = serviceInfo.properties.remove( >> "DefinitionEncoding"); >> try { // we catch classcast etc..., if it fails it is >> not important >> final InputStream is = new >> ByteArrayInputStream(serviceInfo.properties.getProperty("Definition") >> - .getBytes(encoding != null ? encoding.toString() >> : "ISO-8859-1")); >> + .getBytes(encoding != null ? >> encoding.toString() : "ISO-8859-1")); >> final Properties p = new SuperProperties(); >> IO.readProperties(is, p); >> for (final Entry<Object, Object> entry : >> p.entrySet()) { >> final String key = entry.getKey().toString(); >> if (!props.containsKey(key) >> - // never override from Definition, just use >> it to complete the properties set >> - && >> - !(key.equalsIgnoreCase("url") && >> - props.containsKey("JdbcUrl"))) { // with >> @DataSource we can get both, see org.apache.openejb.config.Conv >> ertDataSourceDefinitions.rawDefinition() >> + // never override from Definition, just >> use it to complete the properties set >> + && >> + !(key.equalsIgnoreCase("url") && >> + props.containsKey("JdbcUrl"))) { >> // with @DataSource we can get both, see org.apache.openejb.config.Conv >> ertDataSourceDefinitions.rawDefinition() >> props.put(key, entry.getValue()); >> } >> } >> @@ -3004,10 +3008,10 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> threadPool = Executors.newCachedThreadPool(new >> DaemonThreadFactory(serviceInfo.id + "-worker-")); >> } else { >> threadPool = new ExecutorBuilder() >> - .size(threadPoolSize) >> - .prefix(serviceInfo.id) >> - .threadFactory(new DaemonThreadFactory(serviceInf >> o.id + "-worker-")) >> - .build(new Options(serviceInfo.properties, >> SystemInstance.get().getOptions())); >> + .size(threadPoolSize) >> + .prefix(serviceInfo.id) >> + .threadFactory(new DaemonThreadFactory(serviceInf >> o.id + "-worker-")) >> + .build(new Options(serviceInfo.properties, >> SystemInstance.get().getOptions())); >> logger.info("Thread pool size for '" + serviceInfo.id + >> "' is (" + threadPoolSize + ")"); >> } >> >> @@ -3037,8 +3041,8 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final BootstrapContext bootstrapContext; >> if (transactionManager instanceof >> GeronimoTransactionManager) { >> bootstrapContext = new GeronimoBootstrapContext(Geron >> imoWorkManager.class.cast(workManager), >> - (GeronimoTransactionManager) transactionManager, >> - (GeronimoTransactionManager) transactionManager); >> + (GeronimoTransactionManager) transactionManager, >> + (GeronimoTransactionManager) transactionManager); >> } else if (transactionManager instanceof XATerminator) { >> bootstrapContext = new SimpleBootstrapContext(workManager, >> (XATerminator) transactionManager); >> } else { >> @@ -3103,7 +3107,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> // init cm if needed >> final Object eagerInit = unset.remove("eagerInit"); >> if (eagerInit != null && eagerInit instanceof String && >> "true".equalsIgnoreCase((String) eagerInit) >> - && connectionManager instanceof >> AbstractConnectionManager) { >> + && connectionManager instanceof >> AbstractConnectionManager) { >> try { >> ((AbstractConnectionManager) >> connectionManager).doStart(); >> try { >> @@ -3448,7 +3452,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Assembler assembler = >> SystemInstance.get().getComponent(Assembler.class); >> if (assembler != null) { >> logger = assembler.logger; >> - }else { >> + } else { >> System.err.println("Assembler has not been >> initialized"); >> } >> } >> @@ -3583,7 +3587,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Assembler assembler = >> SystemInstance.get().getComponent(Assembler.class); >> if (assembler != null) { >> assembler.logger.info("assembler.noAgent"); >> - }else { >> + } else { >> System.err.println("destroy: Assembler not >> initialized: JAVA AGENT NOT INSTALLED"); >> } >> } >> @@ -3737,7 +3741,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final Assembler component = >> SystemInstance.get().getComponent(Assembler.class); >> if (component != null) { >> component.logger.error(e.getMessage(), e); >> - }else { >> + } else { >> System.err.println("" + e.getMessage()); >> } >> } >> @@ -3759,7 +3763,7 @@ public class Assembler extends AssemblerTool >> implements org.apache.openejb.spi.A >> final ContainerSystem component = >> SystemInstance.get().getComponent(ContainerSystem.class); >> if (component != null) { >> return component.getJNDIContext().lookup(name); >> - }else { >> + } else { >> throw new Exception("ContainerSystem is not >> initialized"); >> } >> } catch (final Exception e) { >> >>
