TOMEE-1806 patch from Javier Pedrido + ensuring we can find managed connection during eviction
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/0fd8e6ca Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/0fd8e6ca Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/0fd8e6ca Branch: refs/heads/tomee-1.7.x Commit: 0fd8e6cab662b6f53eddb6c42651b3378a4b62a2 Parents: 446e785 Author: Romain manni-Bucau <rmannibu...@gmail.com> Authored: Thu May 12 10:10:32 2016 +0200 Committer: Jonathan Gallimore <j...@jrg.me.uk> Committed: Mon May 30 21:24:48 2016 +0100 ---------------------------------------------------------------------- .../openejb/resource/AutoConnectionTracker.java | 5 + .../GeronimoConnectionManagerFactory.java | 105 +++++++---- .../GeronimoConnectionManagerFactoryTest.java | 184 +++++++++++++++++++ 3 files changed, 261 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/0fd8e6ca/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java index 915e7d9..1c14c5e 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java @@ -31,6 +31,7 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Proxy; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -38,6 +39,10 @@ public class AutoConnectionTracker implements ConnectionTracker { private final ConcurrentMap<ManagedConnectionInfo, ProxyPhantomReference> references = new ConcurrentHashMap<ManagedConnectionInfo, ProxyPhantomReference>(); private final ReferenceQueue referenceQueue = new ReferenceQueue(); + public Set<ManagedConnectionInfo> connections() { + return references.keySet(); + } + /** * Releases any managed connections held by a garbage collected connection proxy. * http://git-wip-us.apache.org/repos/asf/tomee/blob/0fd8e6ca/container/openejb-core/src/main/java/org/apache/openejb/resource/GeronimoConnectionManagerFactory.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/GeronimoConnectionManagerFactory.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/GeronimoConnectionManagerFactory.java index 20ae9ce..9f22091 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/GeronimoConnectionManagerFactory.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/GeronimoConnectionManagerFactory.java @@ -78,10 +78,19 @@ public class GeronimoConnectionManagerFactory { private int poolMaxSize = 10; private int poolMinSize; private boolean allConnectionsEqual = true; + private boolean assumeOneMatch = false; private int connectionMaxWaitMilliseconds = 5000; private int connectionMaxIdleMinutes = 15; - private int validationInterval = -1; private ManagedConnectionFactory mcf; + private int validationIntervalMs = -1; + + public boolean isAssumeOneMatch() { + return assumeOneMatch; + } + + public void setAssumeOneMatch(final boolean assumeOneMatch) { + this.assumeOneMatch = assumeOneMatch; + } public ManagedConnectionFactory getMcf() { return mcf; @@ -196,19 +205,18 @@ public class GeronimoConnectionManagerFactory { } public int getValidationInterval() { - return validationInterval; + return validationIntervalMs < 0 ? -1 : (int) TimeUnit.MILLISECONDS.toMinutes(validationIntervalMs); } public void setValidationInterval(final int validationInterval) { - this.validationInterval = validationInterval; + this.validationIntervalMs = validationInterval < 0 ? -1 : (int) TimeUnit.MINUTES.toMillis(validationInterval); } public void setValidationInterval(final Duration validationInterval) { if (validationInterval.getUnit() == null) { validationInterval.setUnit(TimeUnit.MINUTES); } - final long minutes = TimeUnit.MINUTES.convert(validationInterval.getTime(), validationInterval.getUnit()); - setValidationInterval((int) minutes); + validationIntervalMs = (int) validationInterval.getUnit().toMillis(validationInterval.getTime()); } public GenericConnectionManager create() { @@ -234,18 +242,24 @@ public class GeronimoConnectionManagerFactory { if (txSupport.isRecoverable()) { throw new OpenEJBRuntimeException("currently recoverable tx support (xa) needs a geronimo tx manager"); } - tm = new SimpleRecoverableTransactionManager(transactionManager); + tm = new SimpleRecoverableTransactionManager(transactionManager, name); } - if (validationInterval >= 0 && mcf instanceof ValidatingManagedConnectionFactory) { - return new ValidatingGenericConnectionManager(txSupport, poolingSupport, - null, new AutoConnectionTracker(), tm, - mcf, name, classLoader, validationInterval); + final GenericConnectionManager mgr; + if (validationIntervalMs >= 0 && mcf instanceof ValidatingManagedConnectionFactory) { + if (name == null) { + name = getClass().getSimpleName(); + } + mgr = new ValidatingGenericConnectionManager(txSupport, poolingSupport, + null, new AutoConnectionTracker(), tm, + mcf, name, classLoader, validationIntervalMs); + } else { + mgr = new GenericConnectionManager(txSupport, poolingSupport, + null, new AutoConnectionTracker(), tm, + mcf, name, classLoader); } - return new GenericConnectionManager(txSupport, poolingSupport, - null, new AutoConnectionTracker(), tm, - mcf, name, classLoader); + return mgr; } private TransactionSupport createTransactionSupport() { @@ -271,12 +285,12 @@ public class GeronimoConnectionManagerFactory { // unpartitioned pool return new SinglePool(poolMaxSize, - poolMinSize, - connectionMaxWaitMilliseconds, - connectionMaxIdleMinutes, - allConnectionsEqual, - !allConnectionsEqual, - false); + poolMinSize, + connectionMaxWaitMilliseconds, + connectionMaxIdleMinutes, + allConnectionsEqual, + !allConnectionsEqual, + assumeOneMatch); } else if ("by-connector-properties".equalsIgnoreCase(partitionStrategy)) { @@ -287,7 +301,7 @@ public class GeronimoConnectionManagerFactory { connectionMaxIdleMinutes, allConnectionsEqual, !allConnectionsEqual, - false, + assumeOneMatch, true, false); } else if ("by-subject".equalsIgnoreCase(partitionStrategy)) { @@ -299,19 +313,21 @@ public class GeronimoConnectionManagerFactory { connectionMaxIdleMinutes, allConnectionsEqual, !allConnectionsEqual, - false, + assumeOneMatch, false, true); - } else { - throw new IllegalArgumentException("Unknown partition strategy " + partitionStrategy); } + + throw new IllegalArgumentException("Unknown partition strategy " + partitionStrategy); } private class SimpleRecoverableTransactionManager implements RecoverableTransactionManager { private final TransactionManager delegate; + private final String name; - public SimpleRecoverableTransactionManager(final TransactionManager transactionManager) { - delegate = transactionManager; + public SimpleRecoverableTransactionManager(final TransactionManager transactionManager, final String name) { + this.delegate = transactionManager; + this.name = name; } @Override @@ -320,10 +336,17 @@ public class GeronimoConnectionManagerFactory { } public void registerNamedXAResourceFactory(final NamedXAResourceFactory namedXAResourceFactory) { + if ((name == null && namedXAResourceFactory == null || (namedXAResourceFactory != null && namedXAResourceFactory.getName() == null)) || + (name != null && namedXAResourceFactory != null && name.equals(namedXAResourceFactory.getName()))) { + return; + } throw new UnsupportedOperationException(); } public void unregisterNamedXAResourceFactory(final String namedXAResourceFactoryName) { + if ((name == null && namedXAResourceFactoryName == null) || (name != null && name.equals(namedXAResourceFactoryName))) { + return; + } throw new UnsupportedOperationException(); } @@ -382,7 +405,9 @@ public class GeronimoConnectionManagerFactory { private final ReadWriteLock lock; private final Object pool; - public ValidatingGenericConnectionManager(final TransactionSupport txSupport, final PoolingSupport poolingSupport, final SubjectSource o, final AutoConnectionTracker autoConnectionTracker, final RecoverableTransactionManager tm, final ManagedConnectionFactory mcf, final String name, final ClassLoader classLoader, final long interval) { + public ValidatingGenericConnectionManager(final TransactionSupport txSupport, final PoolingSupport poolingSupport, final SubjectSource o, + final AutoConnectionTracker autoConnectionTracker, final RecoverableTransactionManager tm, + final ManagedConnectionFactory mcf, final String name, final ClassLoader classLoader, final long interval) { super(txSupport, poolingSupport, o, autoConnectionTracker, tm, mcf, name, classLoader); validationInterval = interval; @@ -414,14 +439,14 @@ public class GeronimoConnectionManagerFactory { Object foundPool = null; if (current instanceof AbstractSinglePoolConnectionInterceptor) { - foundPool = Reflections.get(stack, "pool"); + foundPool = Reflections.get(current, "pool"); } else if (current instanceof MultiPoolConnectionInterceptor) { log.warn("validation on stack " + stack + " not supported"); } this.pool = foundPool; if (pool != null) { - validatingTask = new ValidatingTask(current, lock, pool); + validatingTask = new ValidatingTask(current, lock, pool, autoConnectionTracker); } else { validatingTask = null; } @@ -447,11 +472,18 @@ public class GeronimoConnectionManagerFactory { private final ConnectionInterceptor stack; private final ReadWriteLock lock; private final Object pool; + private final AutoConnectionTracker autoConnectionTracker; - public ValidatingTask(final ConnectionInterceptor stack, final ReadWriteLock lock, final Object pool) { + public ValidatingTask(final ConnectionInterceptor stack, final ReadWriteLock lock, final Object pool, + final AutoConnectionTracker autoConnectionTracker) { this.stack = stack; this.lock = lock; this.pool = pool; + this.autoConnectionTracker = autoConnectionTracker; + + if (!SinglePoolConnectionInterceptor.class.isInstance(stack) && !SinglePoolMatchAllConnectionInterceptor.class.isInstance(stack)) { + log.info("stack " + stack + " currently not supported, only AutoConnectionTracker ref will be used for validation"); + } } @Override @@ -463,15 +495,17 @@ public class GeronimoConnectionManagerFactory { try { final Map<ManagedConnection, ManagedConnectionInfo> connections; if (stack instanceof SinglePoolConnectionInterceptor) { - connections = new HashMap<ManagedConnection, ManagedConnectionInfo>(); + connections = new HashMap<>(); for (final ManagedConnectionInfo info : (List<ManagedConnectionInfo>) pool) { connections.put(info.getManagedConnection(), info); } } else if (stack instanceof SinglePoolMatchAllConnectionInterceptor) { connections = (Map<ManagedConnection, ManagedConnectionInfo>) pool; } else { - log.warn("stack " + stack + " currently not supported"); - return; + connections = new HashMap<>(); + } + for (final ManagedConnectionInfo info : autoConnectionTracker.connections()) { + connections.put(info.getManagedConnection(), info); } // destroy invalid connections @@ -480,7 +514,12 @@ public class GeronimoConnectionManagerFactory { .getInvalidConnections(connections.keySet()); if (invalids != null) { for (final ManagedConnection invalid : invalids) { - stack.returnConnection(new ConnectionInfo(connections.get(invalid)), ConnectionReturnAction.DESTROY); + final ManagedConnectionInfo mci = connections.get(invalid); + if (mci != null) { + stack.returnConnection(new ConnectionInfo(mci), ConnectionReturnAction.DESTROY); + continue; + } + log.error("Can't find " + invalid + " in " + pool); } } } catch (final ResourceException e) { http://git-wip-us.apache.org/repos/asf/tomee/blob/0fd8e6ca/container/openejb-core/src/test/java/org/apache/openejb/resource/GeronimoConnectionManagerFactoryTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/GeronimoConnectionManagerFactoryTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/GeronimoConnectionManagerFactoryTest.java new file mode 100644 index 0000000..e2a5646 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/GeronimoConnectionManagerFactoryTest.java @@ -0,0 +1,184 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * 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. + */ +package org.apache.openejb.resource; + +import org.apache.geronimo.connector.outbound.GenericConnectionManager; +import org.apache.geronimo.transaction.manager.TransactionManagerImpl; +import org.apache.openejb.util.Duration; +import org.junit.Test; + +import javax.resource.ResourceException; +import javax.resource.spi.ConnectionEventListener; +import javax.resource.spi.ConnectionManager; +import javax.resource.spi.ConnectionRequestInfo; +import javax.resource.spi.LocalTransaction; +import javax.resource.spi.ManagedConnection; +import javax.resource.spi.ManagedConnectionFactory; +import javax.resource.spi.ManagedConnectionMetaData; +import javax.resource.spi.ValidatingManagedConnectionFactory; +import javax.security.auth.Subject; +import javax.transaction.xa.XAResource; +import java.io.PrintWriter; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.lang.Thread.sleep; +import static org.junit.Assert.assertTrue; + +public class GeronimoConnectionManagerFactoryTest { + @Test // ensure we don't have an exception TOMEE-1806 + public void eviction() throws Exception { + final MyMcf mcf = new MyMcf(); + + final GeronimoConnectionManagerFactory factory = new GeronimoConnectionManagerFactory(); + factory.setValidationInterval(new Duration("1 second")); + factory.setTransactionSupport("local"); + factory.setMcf(mcf); + factory.setPooling(true); + factory.setPartitionStrategy("none"); + factory.setTransactionManager(new TransactionManagerImpl()); + factory.setPoolMinSize(1); + factory.setAllConnectionsEqual(false); + final GenericConnectionManager mgr = factory.create(); + mgr.doStart(); + try { + mgr.allocateConnection(mcf, new ConnectionRequestInfo() { // just to use it + }); + sleep(2500); + assertTrue(mcf.evicted.get()); + assertTrue(mcf.destroyed.get()); + } finally { + mgr.doStop(); + } + } + + public static class MyMcf implements ManagedConnectionFactory, ValidatingManagedConnectionFactory { + private final Set<ManagedConnection> connections = new HashSet<>(); + private final AtomicBoolean evicted = new AtomicBoolean(false); + private final AtomicBoolean destroyed = new AtomicBoolean(false); + + @Override + public Object createConnectionFactory(final ConnectionManager cxManager) throws ResourceException { + return null; + } + + @Override + public Object createConnectionFactory() throws ResourceException { + return null; + } + + @Override + public ManagedConnection createManagedConnection(final Subject subject, final ConnectionRequestInfo cxRequestInfo) throws ResourceException { + return new ManagedConnection() { + @Override + public Object getConnection(Subject subject, ConnectionRequestInfo cxRequestInfo) throws ResourceException { + connections.add(this); + return this; + } + + @Override + public void destroy() throws ResourceException { + connections.remove(this); + destroyed.set(true); + } + + @Override + public void cleanup() throws ResourceException { + // no-op + } + + @Override + public void associateConnection(Object connection) throws ResourceException { + // no-op + } + + @Override + public void addConnectionEventListener(ConnectionEventListener listener) { + // no-op + } + + @Override + public void removeConnectionEventListener(ConnectionEventListener listener) { + // no-op + } + + @Override + public XAResource getXAResource() throws ResourceException { + return null; + } + + @Override + public LocalTransaction getLocalTransaction() throws ResourceException { + return new LocalTransaction() { + @Override + public void begin() throws ResourceException { + + } + + @Override + public void commit() throws ResourceException { + + } + + @Override + public void rollback() throws ResourceException { + + } + }; + } + + @Override + public ManagedConnectionMetaData getMetaData() throws ResourceException { + return null; + } + + @Override + public void setLogWriter(PrintWriter out) throws ResourceException { + // no-op + } + + @Override + public PrintWriter getLogWriter() throws ResourceException { + return null; + } + }; + } + + @Override + public ManagedConnection matchManagedConnections(final Set connectionSet, final Subject subject, + final ConnectionRequestInfo cxRequestInfo) throws ResourceException { + return null; + } + + @Override + public void setLogWriter(PrintWriter out) throws ResourceException { + // no-op + } + + @Override + public PrintWriter getLogWriter() throws ResourceException { + return null; + } + + @Override + public Set getInvalidConnections(final Set connectionSet) throws ResourceException { + evicted.set(true); + return connections; + } + } +}
