Fix rare race condition in OpenEjbContainer This was a mix of the runtime, shutdownhook, & finalizer all calling close on the unsychronized container methods.
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/d68ba480 Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/d68ba480 Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/d68ba480 Branch: refs/heads/tomee-1.7.x Commit: d68ba480d132c74d5c1b086e7feb4b40413c2786 Parents: 7341e10 Author: AndyGee <[email protected]> Authored: Fri Feb 19 17:02:42 2016 +0100 Committer: AndyGee <[email protected]> Committed: Fri Feb 19 17:02:42 2016 +0100 ---------------------------------------------------------------------- .../main/java/org/apache/openejb/OpenEJB.java | 79 +++-- .../org/apache/openejb/OpenEjbContainer.java | 334 +++++++++++-------- .../openejb/core/LocalInitialContext.java | 6 +- .../org/superbiz/injection/secure/Movies.java | 9 +- .../superbiz/injection/secure/MovieTest.java | 40 ++- .../apache/tomee/RemoteTomEEEJBContainer.java | 156 +++++---- .../tomee/embedded/EmbeddedTomEEContainer.java | 238 +++++++------ 7 files changed, 480 insertions(+), 382 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/container/openejb-core/src/main/java/org/apache/openejb/OpenEJB.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/OpenEJB.java b/container/openejb-core/src/main/java/org/apache/openejb/OpenEJB.java index a37ed38..b0cc192 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/OpenEJB.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/OpenEJB.java @@ -103,8 +103,8 @@ public final class OpenEJB { final Logger logger2 = Logger.getInstance(LogCategory.OPENEJB, "org.apache.openejb.util.resources"); final String[] bannerValues = new String[]{ - null, versionInfo.getUrl(), new Date().toString(), versionInfo.getCopyright(), - versionInfo.getVersion(), versionInfo.getDate(), versionInfo.getTime(), null + null, versionInfo.getUrl(), new Date().toString(), versionInfo.getCopyright(), + versionInfo.getVersion(), versionInfo.getDate(), versionInfo.getTime(), null }; for (int i = 0; i < bannerValues.length; i++) { if (bannerValues[i] == null) { @@ -173,9 +173,9 @@ public final class OpenEJB { if (containerSystem.containers().length > 0) { final Container[] c = containerSystem.containers(); logger.debug("startup.debugContainersType"); - for (int i = 0; i < c.length; i++) { + for (final Container aC : c) { String entry = " "; - switch (c[i].getContainerType()) { + switch (aC.getContainerType()) { case BMP_ENTITY: entry += "BMP ENTITY "; break; @@ -191,43 +191,48 @@ public final class OpenEJB { case MESSAGE_DRIVEN: entry += "MESSAGE "; break; + case SINGLETON: + entry += "SINGLETON "; + break; } - entry += c[i].getContainerID(); + entry += aC.getContainerID(); logger.debug("startup.debugEntry", entry); } } - logger.debug("startup.debugDeployments", containerSystem.deployments().length); - if (containerSystem.deployments().length > 0) { - logger.debug("startup.debugDeploymentsType"); - final BeanContext[] d = containerSystem.deployments(); - for (int i = 0; i < d.length; i++) { - String entry = " "; - switch (d[i].getComponentType()) { - case BMP_ENTITY: - entry += "BMP_ENTITY "; - break; - case CMP_ENTITY: - entry += "CMP_ENTITY "; - break; - case STATEFUL: - entry += "STATEFUL "; - break; - case MANAGED: - entry += "MANAGED "; - break; - case STATELESS: - entry += "STATELESS "; - break; - case SINGLETON: - entry += "SINGLETON "; - break; - case MESSAGE_DRIVEN: - entry += "MESSAGE "; - break; + if (logger.isDebugEnabled()) { + logger.debug("startup.debugDeployments", containerSystem.deployments().length); + if (containerSystem.deployments().length > 0) { + logger.debug("startup.debugDeploymentsType"); + final BeanContext[] d = containerSystem.deployments(); + for (final BeanContext aD : d) { + String entry = " "; + switch (aD.getComponentType()) { + case BMP_ENTITY: + entry += "BMP_ENTITY "; + break; + case CMP_ENTITY: + entry += "CMP_ENTITY "; + break; + case STATEFUL: + entry += "STATEFUL "; + break; + case MANAGED: + entry += "MANAGED "; + break; + case STATELESS: + entry += "STATELESS "; + break; + case SINGLETON: + entry += "SINGLETON "; + break; + case MESSAGE_DRIVEN: + entry += "MESSAGE "; + break; + } + entry += aD.getDeploymentID(); + logger.debug("startup.debugEntry", entry); } - entry += d[i].getDeploymentID(); - logger.debug("startup.debugEntry", entry); } } } @@ -261,7 +266,7 @@ public final class OpenEJB { } } - public static void destroy() { + public static synchronized void destroy() { final Assembler assembler = SystemInstance.get().getComponent(Assembler.class); @@ -289,7 +294,7 @@ public final class OpenEJB { /** * 2 usages */ - public static void init(final Properties initProps, final ApplicationServer appServer) throws OpenEJBException { + public static synchronized void init(final Properties initProps, final ApplicationServer appServer) throws OpenEJBException { if (isInitialized()) { if (instance != null) { final String msg = messages.message("startup.alreadyInitialized"); http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/container/openejb-core/src/main/java/org/apache/openejb/OpenEjbContainer.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/OpenEjbContainer.java b/container/openejb-core/src/main/java/org/apache/openejb/OpenEjbContainer.java index 803b6cf..f958728 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/OpenEjbContainer.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/OpenEjbContainer.java @@ -79,6 +79,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.LogManager; import static org.apache.openejb.cdi.ScopeHelper.startContexts; @@ -88,6 +89,7 @@ import static org.apache.openejb.cdi.ScopeHelper.stopContexts; * @version $Rev$ $Date$ */ public final class OpenEjbContainer extends EJBContainer { + static { // if tomee embedded was ran we'll lost log otherwise final String logManger = System.getProperty("java.util.logging.manager"); @@ -107,11 +109,16 @@ public final class OpenEjbContainer extends EJBContainer { } } + /** + * Used to synchronize the container create and close methods + */ + private static final ReentrantLock LOCK = new ReentrantLock(); + public static final String OPENEJB_EMBEDDED_REMOTABLE = "openejb.embedded.remotable"; public static final String OPENEJB_EJBCONTAINER_CLOSE = "openejb.ejbcontainer.close"; public static final String OPENEJB_EJBCONTAINER_CLOSE_SINGLE = "single-jvm"; - private static OpenEjbContainer instance; + private static volatile OpenEjbContainer instance; private static Logger logger; // initialized lazily to get the logging config from properties private ServiceManagerProxy serviceManager; @@ -144,7 +151,9 @@ public final class OpenEjbContainer extends EJBContainer { @Override protected void finalize() throws Throwable { try { - this.close(); + if(this.equals(instance)) { + this.close(); + } } catch (final Exception e) { //no-op } finally { @@ -152,52 +161,57 @@ public final class OpenEjbContainer extends EJBContainer { } } - @Override - public void close() { - if (isSingleClose()) { - return; - } - doClose(); - } - private static boolean isSingleClose() { return OPENEJB_EJBCONTAINER_CLOSE_SINGLE.equals(SystemInstance.get().getProperty(OPENEJB_EJBCONTAINER_CLOSE, "by-invocation")); } - private synchronized void doClose() { - if (instance == null) { - return; - } + private void doClose() { + + final ReentrantLock lock = LOCK; + lock.lock(); - if (serviceManager != null) { - serviceManager.stop(); + final StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); + for (final StackTraceElement element : stackTrace) { + logger().info("Called by: " + element); } + try { - globalJndiContext.close(); - } catch (final NamingException e) { - throw new IllegalStateException(e); - } + if (instance == null) { + return; + } - final Assembler assembler = SystemInstance.get().getComponent(Assembler.class); - if (assembler != null) { - for (final AppInfo info : assembler.getDeployedApplications()) { - try { - assembler.destroyApplication(info); - } catch (final UndeployException e) { - logger().error(e.getMessage(), e); + if (serviceManager != null) { + serviceManager.stop(); + } + try { + globalJndiContext.close(); + } catch (final NamingException e) { + throw new IllegalStateException(e); + } + + final Assembler assembler = SystemInstance.get().getComponent(Assembler.class); + if (assembler != null) { + for (final AppInfo info : assembler.getDeployedApplications()) { + try { + assembler.destroyApplication(info); + } catch (final UndeployException e) { + logger().error(e.getMessage(), e); + } } } - } - try { - stopContexts(webBeanContext.getContextsService(), servletContext, session); - } catch (final Exception e) { - logger().warning("can't stop all CDI contexts", e); - } + try { + stopContexts(webBeanContext.getContextsService(), servletContext, session); + } catch (final Exception e) { + logger().warning("can't stop all CDI contexts", e); + } - logger().info("Destroying OpenEJB container"); - OpenEJB.destroy(); - instance = null; + logger().info("Destroying OpenEJB container"); + OpenEJB.destroy(); + instance = null; + } finally { + lock.unlock(); + } } @Override @@ -226,174 +240,198 @@ public final class OpenEjbContainer extends EJBContainer { return logger; } + @Override + public void close() { + + final ReentrantLock lock = LOCK; + lock.lock(); + + try { + if (isSingleClose()) { + return; + } + doClose(); + } finally { + lock.unlock(); + } + } + public static class Provider implements EJBContainerProvider { public static final String OPENEJB_ADDITIONNAL_CALLERS_KEY = "openejb.additionnal.callers"; @Override public EJBContainer createEJBContainer(Map<?, ?> map) { - if (map == null) { // JBoss EJB API pass null when calling EJBContainer.createEJBContainer() - map = new HashMap<Object, Object>(); - } - if (isOtherProvider(map)) { - return null; - } + final ReentrantLock lock = LOCK; + lock.lock(); - if (instance != null || OpenEJB.isInitialized()) { - if (!isSingleClose()) { - logger().info("EJBContainer already initialized. Call ejbContainer.close() to allow reinitialization"); + try { + if (map == null) { // JBoss EJB API pass null when calling EJBContainer.createEJBContainer() + map = new HashMap<Object, Object>(); } - return instance; - } - - try { - // reset to be able to run this container then tomee one etc... - if (System.getProperties().containsKey(Context.URL_PKG_PREFIXES)) { - System.getProperties().remove(Context.URL_PKG_PREFIXES); + if (isOtherProvider(map)) { + return null; } - final Properties properties = new Properties(); - properties.putAll(map); + if (instance != null || OpenEJB.isInitialized()) { + if (!isSingleClose()) { + logger().info("EJBContainer already initialized. Call ejbContainer.close() to allow reinitialization"); + } + return instance; + } - SystemInstance.reset(); - SystemInstance.init(properties); - SystemInstance.get().setProperty("openejb.embedded", "true"); - SystemInstance.get().setProperty(EJBContainer.class.getName(), "true"); - if (SystemInstance.get().getComponent(ParentClassLoaderFinder.class) == null) { - ClassLoader tccl = Thread.currentThread().getContextClassLoader(); - if (tccl == null) { - tccl = OpenEjbContainer.class.getClassLoader(); + try { + // reset to be able to run this container then tomee one etc... + if (System.getProperties().containsKey(Context.URL_PKG_PREFIXES)) { + System.getProperties().remove(Context.URL_PKG_PREFIXES); } - SystemInstance.get().setComponent(ParentClassLoaderFinder.class, new ProvidedClassLoaderFinder(tccl)); - } - OptionsLog.install(); + final Properties properties = new Properties(); + properties.putAll(map); - OpenEJB.init(properties); + SystemInstance.reset(); + SystemInstance.init(properties); + SystemInstance.get().setProperty("openejb.embedded", "true"); + SystemInstance.get().setProperty(EJBContainer.class.getName(), "true"); - Core.warmup(); // don't do it too eagerly to avoid to not have properties + if (SystemInstance.get().getComponent(ParentClassLoaderFinder.class) == null) { + ClassLoader tccl = Thread.currentThread().getContextClassLoader(); + if (tccl == null) { + tccl = OpenEjbContainer.class.getClassLoader(); + } + SystemInstance.get().setComponent(ParentClassLoaderFinder.class, new ProvidedClassLoaderFinder(tccl)); + } - DeploymentLoader.reloadAltDD(); // otherwise hard to use multiple altdd with several start/stop in the same JVM + OptionsLog.install(); - final ConfigurationFactory configurationFactory = new ConfigurationFactory(); + OpenEJB.init(properties); + Core.warmup(); // don't do it too eagerly to avoid to not have properties - final AppModule appModule = load(map, configurationFactory); + DeploymentLoader.reloadAltDD(); // otherwise hard to use multiple altdd with several start/stop in the same JVM - final Set<String> callers; - if (map.containsKey(OPENEJB_ADDITIONNAL_CALLERS_KEY)) { - callers = new LinkedHashSet<String>(); - callers.addAll(Arrays.asList(((String) map.get(OPENEJB_ADDITIONNAL_CALLERS_KEY)).split(","))); - } else { - callers = NewLoaderLogic.callers(); - } + final ConfigurationFactory configurationFactory = new ConfigurationFactory(); - final EjbJar ejbJar = new EjbJar(); - final OpenejbJar openejbJar = new OpenejbJar(); - for (final String caller : callers) { + final AppModule appModule = load(map, configurationFactory); - if (!isValid(caller)) { - continue; + final Set<String> callers; + if (map.containsKey(OPENEJB_ADDITIONNAL_CALLERS_KEY)) { + callers = new LinkedHashSet<String>(); + callers.addAll(Arrays.asList(((String) map.get(OPENEJB_ADDITIONNAL_CALLERS_KEY)).split(","))); + } else { + callers = NewLoaderLogic.callers(); } - String name = caller; - if (name.contains("$")) { - name = caller.replace("$", "_"); - } + final EjbJar ejbJar = new EjbJar(); + final OpenejbJar openejbJar = new OpenejbJar(); - final ManagedBean bean = ejbJar.addEnterpriseBean(new ManagedBean(name, caller, true)); - bean.localBean(); + for (final String caller : callers) { - // set it to bean so it can get UserTransaction injection - bean.setTransactionType(TransactionType.BEAN); + if (!isValid(caller)) { + continue; + } - final EjbDeployment ejbDeployment = openejbJar.addEjbDeployment(bean); + String name = caller; + if (name.contains("$")) { + name = caller.replace("$", "_"); + } - // important in case any other deploment id formats are specified - ejbDeployment.setDeploymentId(name); - } + final ManagedBean bean = ejbJar.addEnterpriseBean(new ManagedBean(name, caller, true)); + bean.localBean(); - appModule.getEjbModules().add(new EjbModule(ejbJar, openejbJar)); + // set it to bean so it can get UserTransaction injection + bean.setTransactionType(TransactionType.BEAN); + final EjbDeployment ejbDeployment = openejbJar.addEjbDeployment(bean); - final AppInfo appInfo; - try { + // important in case any other deploment id formats are specified + ejbDeployment.setDeploymentId(name); + } - appInfo = configurationFactory.configureApplication(appModule); + appModule.getEjbModules().add(new EjbModule(ejbJar, openejbJar)); - } catch (final ValidationFailedException e) { - logger().warning("configureApplication.loadFailed", appModule.getModuleId(), e.getMessage()); // DO not include the stacktrace in the message + final AppInfo appInfo; + try { - throw new InvalidApplicationException(e); + appInfo = configurationFactory.configureApplication(appModule); - } catch (final OpenEJBException e) { - // DO NOT REMOVE THE EXCEPTION FROM THIS LOG MESSAGE - // removing this message causes NO messages to be printed when embedded - logger().warning("configureApplication.loadFailed", e, appModule.getModuleId(), e.getMessage()); + } catch (final ValidationFailedException e) { - throw new ConfigureApplicationException(e); - } + logger().warning("configureApplication.loadFailed", appModule.getModuleId(), e.getMessage()); // DO not include the stacktrace in the message - final Assembler assembler = SystemInstance.get().getComponent(Assembler.class); + throw new InvalidApplicationException(e); - if(null == assembler){ - throw new IllegalStateException("Assembler has not been initialized"); - } + } catch (final OpenEJBException e) { + // DO NOT REMOVE THE EXCEPTION FROM THIS LOG MESSAGE + // removing this message causes NO messages to be printed when embedded + logger().warning("configureApplication.loadFailed", e, appModule.getModuleId(), e.getMessage()); - final AppContext appContext; + throw new ConfigureApplicationException(e); + } - try { - appContext = assembler.createApplication(appInfo, appModule.getClassLoader()); - } catch (final ValidationException ve) { - throw ve; - } catch (final Exception e) { - throw new AssembleApplicationException(e); - } + final Assembler assembler = SystemInstance.get().getComponent(Assembler.class); + + if (null == assembler) { + throw new IllegalStateException("Assembler has not been initialized"); + } + + final AppContext appContext; + + try { + appContext = assembler.createApplication(appInfo, appModule.getClassLoader()); + } catch (final ValidationException ve) { + throw ve; + } catch (final Exception e) { + throw new AssembleApplicationException(e); + } - final OpenEjbContainer openEjbContainer = instance = new OpenEjbContainer(map, appContext); - if (isSingleClose()) { - Runtime.getRuntime().addShutdownHook(new Thread() { - @Override - public void run() { - if (instance != null) { - instance.doClose(); + final OpenEjbContainer openEjbContainer = instance = new OpenEjbContainer(map, appContext); + if (isSingleClose()) { + Runtime.getRuntime().addShutdownHook(new Thread() { + @Override + public void run() { + if (instance != null) { + instance.doClose(); + } } - } - }); - } - return openEjbContainer; + }); + } + return openEjbContainer; - } catch (final OpenEJBException e) { + } catch (final OpenEJBException e) { - throw new EJBException(e); + throw new EJBException(e); - } catch (final MalformedURLException e) { + } catch (final MalformedURLException e) { - throw new EJBException(e); + throw new EJBException(e); - } catch (final ValidationException ve) { - throw ve; - } catch (final Exception e) { + } catch (final ValidationException ve) { + throw ve; + } catch (final Exception e) { - if (e instanceof EJBException) { + if (e instanceof EJBException) { - throw (EJBException) e; - } + throw (EJBException) e; + } - throw new InitializationException(e); - } finally { - if (instance == null && OpenEJB.isInitialized()) { - try { - OpenEJB.destroy(); - } catch (final Exception e) { - // no-op + throw new InitializationException(e); + } finally { + if (instance == null && OpenEJB.isInitialized()) { + try { + OpenEJB.destroy(); + } catch (final Exception e) { + // no-op + } } } + } finally { + lock.unlock(); } } @@ -590,7 +628,7 @@ public final class OpenEjbContainer extends EJBContainer { private static boolean isOtherProvider(final Map<?, ?> properties) { final Object provider = properties.get(EJBContainer.PROVIDER); return provider != null && !provider.equals(OpenEjbContainer.class) && !provider.equals(OpenEjbContainer.class.getName()) - && !"openejb".equals(provider); + && !"openejb".equals(provider); } private boolean match(final String s, final File file) { http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/container/openejb-core/src/main/java/org/apache/openejb/core/LocalInitialContext.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/LocalInitialContext.java b/container/openejb-core/src/main/java/org/apache/openejb/core/LocalInitialContext.java index b43570d..64a645c 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/core/LocalInitialContext.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/core/LocalInitialContext.java @@ -141,12 +141,14 @@ public class LocalInitialContext extends ContextWrapper { @SuppressWarnings("unchecked") private void logout() { try { - final SecurityService securityService = SystemInstance.get().getComponent(SecurityService.class); if (clientIdentity != null) { if (logger.isDebugEnabled()) { logger.debug("Logging out: " + clientIdentity); } - securityService.logout(clientIdentity); + final SecurityService securityService = SystemInstance.get().getComponent(SecurityService.class); + if (null != securityService) { + securityService.logout(clientIdentity); + } ClientSecurity.setIdentity(null); } } catch (final LoginException e) { http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/examples/testing-security-4/src/main/java/org/superbiz/injection/secure/Movies.java ---------------------------------------------------------------------- diff --git a/examples/testing-security-4/src/main/java/org/superbiz/injection/secure/Movies.java b/examples/testing-security-4/src/main/java/org/superbiz/injection/secure/Movies.java index b3ba8e8..96d95ff 100644 --- a/examples/testing-security-4/src/main/java/org/superbiz/injection/secure/Movies.java +++ b/examples/testing-security-4/src/main/java/org/superbiz/injection/secure/Movies.java @@ -18,6 +18,7 @@ package org.superbiz.injection.secure; //START SNIPPET: code +import javax.annotation.PreDestroy; import javax.annotation.security.PermitAll; import javax.annotation.security.RolesAllowed; import javax.ejb.Stateful; @@ -27,6 +28,7 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.PersistenceContextType; import javax.persistence.Query; +import javax.persistence.TypedQuery; import java.util.List; @Stateful @@ -36,20 +38,19 @@ public class Movies { private EntityManager entityManager; @RolesAllowed({"Employee", "Manager"}) - public void addMovie(Movie movie) throws Exception { + public void addMovie(final Movie movie) throws Exception { entityManager.persist(movie); } @RolesAllowed({"Manager"}) - public void deleteMovie(Movie movie) throws Exception { + public void deleteMovie(final Movie movie) throws Exception { entityManager.remove(movie); } @PermitAll @TransactionAttribute(TransactionAttributeType.SUPPORTS) public List<Movie> getMovies() throws Exception { - Query query = entityManager.createQuery("SELECT m from Movie as m"); - return query.getResultList(); + return entityManager.createQuery("SELECT m from Movie as m", Movie.class).getResultList(); } } //END SNIPPET: code http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/examples/testing-security-4/src/test/java/org/superbiz/injection/secure/MovieTest.java ---------------------------------------------------------------------- diff --git a/examples/testing-security-4/src/test/java/org/superbiz/injection/secure/MovieTest.java b/examples/testing-security-4/src/test/java/org/superbiz/injection/secure/MovieTest.java index 0e3654b..2483e66 100644 --- a/examples/testing-security-4/src/test/java/org/superbiz/injection/secure/MovieTest.java +++ b/examples/testing-security-4/src/test/java/org/superbiz/injection/secure/MovieTest.java @@ -21,6 +21,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import javax.annotation.PreDestroy; import javax.ejb.EJB; import javax.ejb.EJBAccessException; import javax.ejb.embeddable.EJBContainer; @@ -36,12 +37,13 @@ public class MovieTest { @EJB private Movies movies; - private EJBContainer container; + private volatile EJBContainer container; - private Context getContext(String user, String pass) throws NamingException { - Properties p = new Properties(); + private Context getContext(final String user, final String pass) throws NamingException { + final Properties p = new Properties(); p.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.openejb.core.LocalInitialContextFactory"); p.setProperty("openejb.authentication.realmName", "ScriptLogin"); + p.setProperty("openejb.embedded.initialcontext.close", "destroy"); p.put(Context.SECURITY_PRINCIPAL, user); p.put(Context.SECURITY_CREDENTIALS, pass); @@ -49,21 +51,25 @@ public class MovieTest { } @Before - public void setUp() throws Exception { + public void before() throws Exception { final ClassLoader ctxCl = Thread.currentThread().getContextClassLoader(); System.setProperty("openejb.ScriptLoginModule.scriptURI", ctxCl.getResource("loginscript.js").toExternalForm()); - Properties p = new Properties(); + final Properties p = new Properties(); p.put("movieDatabase", "new://Resource?type=DataSource"); p.put("movieDatabase.JdbcDriver", "org.hsqldb.jdbcDriver"); p.put("movieDatabase.JdbcUrl", "jdbc:hsqldb:mem:moviedb"); this.container = EJBContainer.createEJBContainer(p); - this.container.getContext().bind("inject", this); + try { + this.container.getContext().bind("inject", this); + } catch (final Exception e) { + e.printStackTrace(); + } } @After - public void tearDown() { + public void after() { this.container.close(); } @@ -76,10 +82,10 @@ public class MovieTest { movies.addMovie(new Movie("Joel Coen", "Fargo", 1996)); movies.addMovie(new Movie("Joel Coen", "The Big Lebowski", 1998)); - List<Movie> list = movies.getMovies(); + final List<Movie> list = movies.getMovies(); Assert.assertEquals("List.size()", 3, list.size()); - for (Movie movie : list) { + for (final Movie movie : list) { movies.deleteMovie(movie); } @@ -98,14 +104,14 @@ public class MovieTest { movies.addMovie(new Movie("Joel Coen", "Fargo", 1996)); movies.addMovie(new Movie("Joel Coen", "The Big Lebowski", 1998)); - List<Movie> list = movies.getMovies(); + final List<Movie> list = movies.getMovies(); Assert.assertEquals("List.size()", 3, list.size()); - for (Movie movie : list) { + for (final Movie movie : list) { try { movies.deleteMovie(movie); Assert.fail("Employees should not be allowed to delete"); - } catch (EJBAccessException e) { + } catch (final EJBAccessException e) { // Good, Employees cannot delete things } } @@ -122,21 +128,21 @@ public class MovieTest { try { movies.addMovie(new Movie("Quentin Tarantino", "Reservoir Dogs", 1992)); Assert.fail("Unauthenticated users should not be able to add movies"); - } catch (EJBAccessException e) { + } catch (final EJBAccessException e) { // Good, guests cannot add things } try { movies.deleteMovie(null); Assert.fail("Unauthenticated users should not be allowed to delete"); - } catch (EJBAccessException e) { + } catch (final EJBAccessException e) { // Good, Unauthenticated users cannot delete things } try { // Read access should be allowed movies.getMovies(); - } catch (EJBAccessException e) { + } catch (final EJBAccessException e) { Assert.fail("Read access should be allowed"); } } @@ -146,14 +152,14 @@ public class MovieTest { try { getContext("eddie", "panama"); Assert.fail("supposed to have a login failure here"); - } catch (javax.naming.AuthenticationException e) { + } catch (final javax.naming.AuthenticationException e) { //expected } try { getContext("jimmy", "foxylady"); Assert.fail("supposed to have a login failure here"); - } catch (javax.naming.AuthenticationException e) { + } catch (final javax.naming.AuthenticationException e) { //expected } } http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/tomee/apache-tomee/src/main/java/org/apache/tomee/RemoteTomEEEJBContainer.java ---------------------------------------------------------------------- diff --git a/tomee/apache-tomee/src/main/java/org/apache/tomee/RemoteTomEEEJBContainer.java b/tomee/apache-tomee/src/main/java/org/apache/tomee/RemoteTomEEEJBContainer.java index 67f68b1..60b5583 100644 --- a/tomee/apache-tomee/src/main/java/org/apache/tomee/RemoteTomEEEJBContainer.java +++ b/tomee/apache-tomee/src/main/java/org/apache/tomee/RemoteTomEEEJBContainer.java @@ -38,8 +38,15 @@ import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.concurrent.locks.ReentrantLock; public class RemoteTomEEEJBContainer extends EJBContainer { + + /** + * Used to synchronize the container create and close methods + */ + private static final ReentrantLock LOCK = new ReentrantLock(); + private static RemoteTomEEEJBContainer instance; private RemoteServer container; private InitialContext context; @@ -57,8 +64,15 @@ public class RemoteTomEEEJBContainer extends EJBContainer { @Override public void close() { - instance.container.destroy(); - instance.container = null; + final ReentrantLock lock = LOCK; + lock.lock(); + + try { + instance.container.destroy(); + instance.container = null; + } finally { + lock.unlock(); + } } @Override @@ -71,83 +85,91 @@ public class RemoteTomEEEJBContainer extends EJBContainer { @Override public EJBContainer createEJBContainer(final Map<?, ?> properties) { - final Object provider = properties.get(EJBContainer.PROVIDER); - int ejbContainerProviders = 1; - try { - ejbContainerProviders = ProviderLocator.getServices(EJBContainerProvider.class.getName(), EJBContainer.class, Thread.currentThread().getContextClassLoader()).size(); - } catch (final Exception e) { - // no-op - } - - if ((provider == null && ejbContainerProviders > 1) - || (!RemoteTomEEEJBContainer.class.equals(provider) - && !CONTAINER_NAMES.contains(String.valueOf(provider)))) { - return null; - } - if (instance != null) { - return instance; - } + final ReentrantLock lock = LOCK; + lock.lock(); - final Object modules = properties.get(EJBContainer.MODULES); + try { + final Object provider = properties.get(EJBContainer.PROVIDER); + int ejbContainerProviders = 1; + try { + ejbContainerProviders = ProviderLocator.getServices(EJBContainerProvider.class.getName(), EJBContainer.class, Thread.currentThread().getContextClassLoader()).size(); + } catch (final Exception e) { + // no-op + } - System.getProperties().putAll(properties); - final File home = new File(System.getProperty("openejb.home", "doesn't exist")); - if (!home.exists()) { - throw new IllegalArgumentException("You need to set openejb.home"); - } + if ((provider == null && ejbContainerProviders > 1) + || (!RemoteTomEEEJBContainer.class.equals(provider) + && !CONTAINER_NAMES.contains(String.valueOf(provider)))) { + return null; + } - final QuickServerXmlParser parser = QuickServerXmlParser.parse(new File(home, "conf/server.xml")); - final String remoteEjb = System.getProperty(Context.PROVIDER_URL, "http://"; + parser.host() + ":" + parser.http() + "/tomee/ejb"); + if (instance != null) { + return instance; + } - try { - instance = new RemoteTomEEEJBContainer(); - instance.container = new RemoteServer(); - instance.container.setPortStartup(Integer.parseInt(parser.http())); + final Object modules = properties.get(EJBContainer.MODULES); - try { - instance.container.start(); - } catch (final Exception e) { - instance.container.destroy(); - throw e; + System.getProperties().putAll(properties); + final File home = new File(System.getProperty("openejb.home", "doesn't exist")); + if (!home.exists()) { + throw new IllegalArgumentException("You need to set openejb.home"); } - instance.context = new InitialContext(new Properties() {{ - setProperty(Context.INITIAL_CONTEXT_FACTORY, RemoteInitialContextFactory.class.getName()); - setProperty(Context.PROVIDER_URL, remoteEjb); - }}); - - final Deployer deployer = Deployer.class.cast(instance.context.lookup("openejb/DeployerBusinessRemote")); - - if (modules instanceof File) { - final File file = File.class.cast(modules); - deployFile(deployer, file); - } else if (modules instanceof String) { - final String path = String.class.cast(modules); - final File file = new File(path); - deployFile(deployer, file); - } else if (modules instanceof String[]) { - for (final String path : (String[]) modules) { - deployFile(deployer, new File(path)); + final QuickServerXmlParser parser = QuickServerXmlParser.parse(new File(home, "conf/server.xml")); + final String remoteEjb = System.getProperty(Context.PROVIDER_URL, "http://"; + parser.host() + ":" + parser.http() + "/tomee/ejb"); + + try { + instance = new RemoteTomEEEJBContainer(); + instance.container = new RemoteServer(); + instance.container.setPortStartup(Integer.parseInt(parser.http())); + + try { + instance.container.start(); + } catch (final Exception e) { + instance.container.destroy(); + throw e; } - } else if (modules instanceof File[]) { - for (final File file : (File[]) modules) { + + instance.context = new InitialContext(new Properties() {{ + setProperty(Context.INITIAL_CONTEXT_FACTORY, RemoteInitialContextFactory.class.getName()); + setProperty(Context.PROVIDER_URL, remoteEjb); + }}); + + final Deployer deployer = Deployer.class.cast(instance.context.lookup("openejb/DeployerBusinessRemote")); + + if (modules instanceof File) { + final File file = File.class.cast(modules); + deployFile(deployer, file); + } else if (modules instanceof String) { + final String path = String.class.cast(modules); + final File file = new File(path); deployFile(deployer, file); + } else if (modules instanceof String[]) { + for (final String path : (String[]) modules) { + deployFile(deployer, new File(path)); + } + } else if (modules instanceof File[]) { + for (final File file : (File[]) modules) { + deployFile(deployer, file); + } + } // else suppose already deployed + + return instance; + } catch (final OpenEJBException e) { + throw new EJBException(e); + } catch (final MalformedURLException e) { + throw new EJBException(e); + } catch (final ValidationException ve) { + throw ve; + } catch (final Exception e) { + if (e instanceof EJBException) { + throw (EJBException) e; } - } // else suppose already deployed - - return instance; - } catch (final OpenEJBException e) { - throw new EJBException(e); - } catch (final MalformedURLException e) { - throw new EJBException(e); - } catch (final ValidationException ve) { - throw ve; - } catch (final Exception e) { - if (e instanceof EJBException) { - throw (EJBException) e; + throw new TomEERemoteEJBContainerException("initialization exception", e); } - throw new TomEERemoteEJBContainerException("initialization exception", e); + } finally { + lock.unlock(); } } } http://git-wip-us.apache.org/repos/asf/tomee/blob/d68ba480/tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/EmbeddedTomEEContainer.java ---------------------------------------------------------------------- diff --git a/tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/EmbeddedTomEEContainer.java b/tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/EmbeddedTomEEContainer.java index 964dd5d..7d30a4e 100644 --- a/tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/EmbeddedTomEEContainer.java +++ b/tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/EmbeddedTomEEContainer.java @@ -5,14 +5,14 @@ * The ASF licenses this file to You under the Apache License, Version 2.0 * (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ package org.apache.tomee.embedded; @@ -39,8 +39,15 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReentrantLock; public final class EmbeddedTomEEContainer extends EJBContainer { + + /** + * Used to synchronize the container create and close methods + */ + private static final ReentrantLock LOCK = new ReentrantLock(); + public static final String TOMEE_EJBCONTAINER_HTTP_PORT = "tomee.ejbcontainer.http.port"; private static final AtomicReference<EmbeddedTomEEContainer> tomEEContainer = new AtomicReference<EmbeddedTomEEContainer>(); private static final List<String> CONTAINER_NAMES = Arrays.asList(EmbeddedTomEEContainer.class.getName(), "tomee-embedded", "embedded-tomee"); @@ -52,6 +59,7 @@ public final class EmbeddedTomEEContainer extends EJBContainer { // no-op } + @SuppressWarnings("unused") public Container getDelegate() { return container; } @@ -69,30 +77,38 @@ public final class EmbeddedTomEEContainer extends EJBContainer { @Override public void close() { - final Collection<Exception> errors = new ArrayList<Exception>(); - for (final String id : deployedIds) { - if (tomEEContainer.get().container.getAppContexts(id) != null) { - try { - tomEEContainer.get().container.undeploy(id); - } catch (final Exception ex) { - Logger.getInstance(LogCategory.OPENEJB, EmbeddedTomEEContainer.class).error(ex.getMessage(), ex); - errors.add(ex); + + final ReentrantLock lock = LOCK; + lock.lock(); + + try { + final Collection<Exception> errors = new ArrayList<Exception>(); + for (final String id : deployedIds) { + if (tomEEContainer.get().container.getAppContexts(id) != null) { + try { + tomEEContainer.get().container.undeploy(id); + } catch (final Exception ex) { + Logger.getInstance(LogCategory.OPENEJB, EmbeddedTomEEContainer.class).error(ex.getMessage(), ex); + errors.add(ex); + } } } - } - deployedIds.clear(); + deployedIds.clear(); - try { - tomEEContainer.get().container.close(); - } catch (final Exception ex) { - errors.add(ex); - Logger.getInstance(LogCategory.OPENEJB, EmbeddedTomEEContainer.class).error(ex.getMessage(), ex); - }finally { - tomEEContainer.set(null); - } + try { + tomEEContainer.get().container.close(); + } catch (final Exception ex) { + errors.add(ex); + Logger.getInstance(LogCategory.OPENEJB, EmbeddedTomEEContainer.class).error(ex.getMessage(), ex); + } finally { + tomEEContainer.set(null); + } - if (!errors.isEmpty()) { - throw Exceptions.newEJBException(new TomEERuntimeException(errors.toString())); + if (!errors.isEmpty()) { + throw Exceptions.newEJBException(new TomEERuntimeException(errors.toString())); + } + } finally { + lock.unlock(); } } @@ -104,98 +120,106 @@ public final class EmbeddedTomEEContainer extends EJBContainer { public static class EmbeddedTomEEContainerProvider implements EJBContainerProvider { @Override public EJBContainer createEJBContainer(final Map<?, ?> properties) { - final Object provider = properties.get(EJBContainer.PROVIDER); - int ejbContainerProviders = 1; - try { - ejbContainerProviders = ProviderLocator.getServices(EJBContainerProvider.class.getName(), EJBContainer.class, Thread.currentThread().getContextClassLoader()).size(); - } catch (final Exception e) { - // no-op - } - if ((provider == null && ejbContainerProviders > 1) - || (!EmbeddedTomEEContainer.class.equals(provider) - && !CONTAINER_NAMES.contains(String.valueOf(provider)))) { - return null; - } + final ReentrantLock lock = LOCK; + lock.lock(); - if (tomEEContainer.get() != null) { - return tomEEContainer.get(); - } + try { + final Object provider = properties.get(EJBContainer.PROVIDER); + int ejbContainerProviders = 1; + try { + ejbContainerProviders = ProviderLocator.getServices(EJBContainerProvider.class.getName(), EJBContainer.class, Thread.currentThread().getContextClassLoader()).size(); + } catch (final Exception e) { + // no-op + } - final String appId = (String) properties.get(EJBContainer.APP_NAME); - final Object modules = properties.get(EJBContainer.MODULES); - - tomEEContainer.set(new EmbeddedTomEEContainer()); - final Configuration configuration = new Configuration(); - if (properties.containsKey(TOMEE_EJBCONTAINER_HTTP_PORT)) { - int port; - final Object portValue = properties.get(TOMEE_EJBCONTAINER_HTTP_PORT); - if (portValue instanceof Integer) { - port = (Integer) portValue; - } else if (portValue instanceof String) { - port = Integer.parseInt((String) portValue); - } else { - throw new TomEERuntimeException("port value should be an integer or a string"); + if ((provider == null && ejbContainerProviders > 1) + || (!EmbeddedTomEEContainer.class.equals(provider) + && !CONTAINER_NAMES.contains(String.valueOf(provider)))) { + return null; } - if (port <= 0) { - port = NetworkUtil.getNextAvailablePort(); + + if (tomEEContainer.get() != null) { + return tomEEContainer.get(); } - configuration.setHttpPort(port); - } - System.setProperty(TOMEE_EJBCONTAINER_HTTP_PORT, Integer.toString(configuration.getHttpPort())); - tomEEContainer.get().container.setup(configuration); - try { - tomEEContainer.get().container.start(); - - if (modules instanceof File) { - tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, ((File) modules), appId != null).getId()); - } else if (modules instanceof String) { - tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, new File((String) modules), appId != null).getId()); - } else if (modules instanceof String[]) { - for (final String path : (String[]) modules) { - tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, new File(path), appId != null).getId()); + + final String appId = (String) properties.get(EJBContainer.APP_NAME); + final Object modules = properties.get(EJBContainer.MODULES); + + tomEEContainer.set(new EmbeddedTomEEContainer()); + final Configuration configuration = new Configuration(); + if (properties.containsKey(TOMEE_EJBCONTAINER_HTTP_PORT)) { + int port; + final Object portValue = properties.get(TOMEE_EJBCONTAINER_HTTP_PORT); + if (portValue instanceof Integer) { + port = (Integer) portValue; + } else if (portValue instanceof String) { + port = Integer.parseInt((String) portValue); + } else { + throw new TomEERuntimeException("port value should be an integer or a string"); } - } else if (modules instanceof File[]) { - for (final File file : (File[]) modules) { - tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, file, appId != null).getId()); + if (port <= 0) { + port = NetworkUtil.getNextAvailablePort(); } - } else { - SystemInstance.get().getProperties().putAll(properties); - final Collection<File> files = tomEEContainer.get().container.getConfigurationFactory().getModulesFromClassPath(null, Thread.currentThread().getContextClassLoader()); - if (files.size() == 0) { - try { - tomEEContainer.get().close(); - } catch (final Exception e) { - // no-op + configuration.setHttpPort(port); + } + System.setProperty(TOMEE_EJBCONTAINER_HTTP_PORT, Integer.toString(configuration.getHttpPort())); + tomEEContainer.get().container.setup(configuration); + try { + tomEEContainer.get().container.start(); + + if (modules instanceof File) { + tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, ((File) modules), appId != null).getId()); + } else if (modules instanceof String) { + tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, new File((String) modules), appId != null).getId()); + } else if (modules instanceof String[]) { + for (final String path : (String[]) modules) { + tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, new File(path), appId != null).getId()); + } + } else if (modules instanceof File[]) { + for (final File file : (File[]) modules) { + tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, file, appId != null).getId()); + } + } else { + SystemInstance.get().getProperties().putAll(properties); + final Collection<File> files = tomEEContainer.get().container.getConfigurationFactory().getModulesFromClassPath(null, Thread.currentThread().getContextClassLoader()); + if (files.size() == 0) { + try { + tomEEContainer.get().close(); + } catch (final Exception e) { + // no-op + } + tomEEContainer.set(null); + throw Exceptions.newNoModulesFoundException(); + } + for (final File file : files) { + tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, file, appId != null).getId()); } - tomEEContainer.set(null); - throw Exceptions.newNoModulesFoundException(); - } - for (final File file : files) { - tomEEContainer.get().deployedIds.add(tomEEContainer.get().container.deploy(appId, file, appId != null).getId()); } - } - return tomEEContainer.get(); - } catch (final OpenEJBException e) { - tomEEContainer.get().close(); - throw new EJBException(e); - } catch (final MalformedURLException e) { - tomEEContainer.get().close(); - throw new EJBException(e); - } catch (final ValidationException ve) { - if (tomEEContainer.get() != null) { + return tomEEContainer.get(); + } catch (final OpenEJBException e) { tomEEContainer.get().close(); - } - throw ve; - } catch (final Exception e) { - if (tomEEContainer.get() != null) { + throw new EJBException(e); + } catch (final MalformedURLException e) { tomEEContainer.get().close(); + throw new EJBException(e); + } catch (final ValidationException ve) { + if (tomEEContainer.get() != null) { + tomEEContainer.get().close(); + } + throw ve; + } catch (final Exception e) { + if (tomEEContainer.get() != null) { + tomEEContainer.get().close(); + } + if (e instanceof EJBException) { + throw (EJBException) e; + } + throw new TomEERuntimeException("initialization exception", e); } - if (e instanceof EJBException) { - throw (EJBException) e; - } - throw new TomEERuntimeException("initialization exception", e); + } finally { + lock.unlock(); } } }
