You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by gt...@apache.org on 2012/03/30 00:05:19 UTC
svn commit: r1307142 - in /activemq/trunk/activemq-core/src:
main/java/org/apache/activemq/broker/jmx/
main/java/org/apache/activemq/broker/region/ test/java/org/apache/activemq/
test/java/org/apache/activemq/bugs/
Author: gtully
Date: Thu Mar 29 22:05:18 2012
New Revision: 1307142
URL: http://svn.apache.org/viewvc?rev=1307142&view=rev
Log:
https://issues.apache.org/jira/browse/AMQ-3792: use of the failover transport incorrectly suppresses javax.jms.InvalidClientIDException when clientId is already in use. fixup with test and fix to test dependent on wrong behaviour
Modified:
activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java
activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/RegionBroker.java
activemq/trunk/activemq-core/src/test/java/org/apache/activemq/ReconnectWithSameClientIDTest.java
activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2580Test.java
activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2902Test.java
Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java?rev=1307142&r1=1307141&r2=1307142&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java Thu Mar 29 22:05:18 2012
@@ -62,18 +62,17 @@ public class ManagedTransportConnection
}
}
- public void doStop() throws Exception {
- if (isStarting()) {
- setPendingStop(true);
- return;
- }
- synchronized (this) {
- unregisterMBean(byClientIdName);
- unregisterMBean(byAddressName);
- byClientIdName = null;
- byAddressName = null;
+ @Override
+ public void stopAsync() {
+ if (!isStopping()) {
+ synchronized (this) {
+ unregisterMBean(byClientIdName);
+ unregisterMBean(byAddressName);
+ byClientIdName = null;
+ byAddressName = null;
+ }
}
- super.doStop();
+ super.stopAsync();
}
public Response processAddConnection(ConnectionInfo info) throws Exception {
Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/RegionBroker.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/RegionBroker.java?rev=1307142&r1=1307141&r2=1307142&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/RegionBroker.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/RegionBroker.java Thu Mar 29 22:05:18 2012
@@ -236,17 +236,8 @@ public class RegionBroker extends EmptyB
synchronized (clientIdSet) {
ConnectionContext oldContext = clientIdSet.get(clientId);
if (oldContext != null) {
- if (context.isFaultTolerant() || context.isNetworkConnection()){
- //remove the old connection
- try{
- removeConnection(oldContext, info, new Exception("remove stale client"));
- }catch(Exception e){
- LOG.warn("Failed to remove stale connection ",e);
- }
- }else{
throw new InvalidClientIDException("Broker: " + getBrokerName() + " - Client: " + clientId + " already connected from "
+ oldContext.getConnection().getRemoteAddress());
- }
} else {
clientIdSet.put(clientId, context);
}
Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/ReconnectWithSameClientIDTest.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/ReconnectWithSameClientIDTest.java?rev=1307142&r1=1307141&r2=1307142&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/ReconnectWithSameClientIDTest.java (original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/ReconnectWithSameClientIDTest.java Thu Mar 29 22:05:18 2012
@@ -16,12 +16,17 @@
*/
package org.apache.activemq;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.jms.Connection;
import javax.jms.ConnectionFactory;
import javax.jms.InvalidClientIDException;
import javax.jms.JMSException;
import javax.jms.Session;
+import junit.framework.Test;
+import org.apache.activemq.util.DefaultTestAppender;
+import org.apache.log4j.Appender;
+import org.apache.log4j.spi.LoggingEvent;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -31,33 +36,64 @@ public class ReconnectWithSameClientIDTe
protected Connection connection;
protected boolean transacted;
protected int authMode = Session.AUTO_ACKNOWLEDGE;
+ public boolean useFailover = false;
+
+ public static Test suite() {
+ return suite(ReconnectWithSameClientIDTest.class);
+ }
+
+ public void initCombosForTestReconnectMultipleTimesWithSameClientID() {
+ addCombinationValues("useFailover", new Object[]{Boolean.FALSE, Boolean.TRUE});
+ }
public void testReconnectMultipleTimesWithSameClientID() throws Exception {
- connection = connectionFactory.createConnection();
- useConnection(connection);
- // now lets create another which should fail
- for (int i = 1; i < 11; i++) {
- Connection connection2 = connectionFactory.createConnection();
- try {
- useConnection(connection2);
- fail("Should have thrown InvalidClientIDException on attempt" + i);
- } catch (InvalidClientIDException e) {
- connection2.close();
- LOG.info("Caught expected: " + e);
+ org.apache.log4j.Logger log4jLogger =
+ org.apache.log4j.Logger.getLogger(org.apache.activemq.broker.jmx.ManagedTransportConnection.class);
+ final AtomicBoolean failed = new AtomicBoolean(false);
+
+ Appender appender = new DefaultTestAppender() {
+ @Override
+ public void doAppend(LoggingEvent event) {
+ if (event.getMessage().toString().startsWith("Failed to register MBean")) {
+ LOG.info("received unexpected log message: " + event.getMessage());
+ failed.set(true);
+ }
+ }
+ };
+ log4jLogger.addAppender(appender);
+ try {
+ connection = connectionFactory.createConnection();
+ useConnection(connection);
+
+ // now lets create another which should fail
+ for (int i = 1; i < 11; i++) {
+ Connection connection2 = connectionFactory.createConnection();
+ try {
+ useConnection(connection2);
+ fail("Should have thrown InvalidClientIDException on attempt" + i);
+ } catch (InvalidClientIDException e) {
+ LOG.info("Caught expected: " + e);
+ } finally {
+ connection2.close();
+ }
}
- }
- // now lets try closing the original connection and creating a new
- // connection with the same ID
- connection.close();
- connection = connectionFactory.createConnection();
- useConnection(connection);
+ // now lets try closing the original connection and creating a new
+ // connection with the same ID
+ connection.close();
+ connection = connectionFactory.createConnection();
+ useConnection(connection);
+ } finally {
+ log4jLogger.removeAppender(appender);
+ }
+ assertFalse("failed on unexpected log event", failed.get());
}
@Override
protected ConnectionFactory createConnectionFactory() throws Exception {
- return new ActiveMQConnectionFactory(broker.getTransportConnectors().get(0).getPublishableConnectString());
+ return new ActiveMQConnectionFactory((useFailover ? "failover:" : "") +
+ broker.getTransportConnectors().get(0).getPublishableConnectString());
}
protected void setUp() throws Exception {
Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2580Test.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2580Test.java?rev=1307142&r1=1307141&r2=1307142&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2580Test.java (original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2580Test.java Thu Mar 29 22:05:18 2012
@@ -48,7 +48,6 @@ public class AMQ2580Test extends TestSup
private Session session;
private MessageProducer producer;
private ConnectionFactory connectionFactory;
- private TopicConnection topicConnection;
private BrokerService service;
public static Test suite() {
@@ -195,8 +194,8 @@ public class AMQ2580Test extends TestSup
}
private void initTopic() throws JMSException {
- topicConnection = (TopicConnection) connectionFactory.createConnection();
- TopicSession topicSession = topicConnection.createTopicSession(false, Session.AUTO_ACKNOWLEDGE);
+ initConnection();
+ TopicSession topicSession = connection.createTopicSession(false, Session.AUTO_ACKNOWLEDGE);
topic = topicSession.createTopic(TOPIC_NAME);
}
}
Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2902Test.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2902Test.java?rev=1307142&r1=1307141&r2=1307142&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2902Test.java (original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ2902Test.java Thu Mar 29 22:05:18 2012
@@ -26,14 +26,14 @@ import org.apache.activemq.broker.Transp
import org.apache.activemq.transport.TransportDisposedIOException;
import org.apache.activemq.util.DefaultTestAppender;
import org.apache.log4j.Appender;
-import org.apache.log4j.Layout;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
-import org.apache.log4j.spi.ErrorHandler;
-import org.apache.log4j.spi.Filter;
import org.apache.log4j.spi.LoggingEvent;
+import org.slf4j.LoggerFactory;
public class AMQ2902Test extends TestCase {
+ private static final org.slf4j.Logger LOG = LoggerFactory.getLogger(AMQ2580Test.class);
+
final AtomicBoolean gotExceptionInLog = new AtomicBoolean(Boolean.FALSE);
final AtomicBoolean failedToFindMDC = new AtomicBoolean(Boolean.FALSE);
@@ -42,6 +42,7 @@ public class AMQ2902Test extends TestCas
public void doAppend(LoggingEvent event) {
if (event.getThrowableInformation() != null
&& event.getThrowableInformation().getThrowable() instanceof TransportDisposedIOException) {
+ LOG.error("got event: " + event + ", ex:" + event.getThrowableInformation().getThrowable(), event.getThrowableInformation().getThrowable());
gotExceptionInLog.set(Boolean.TRUE);
}
if (event.getMDC("activemq.broker") == null) {