From fa83413bbca3b76e810292939c18ab20b026142e Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Mon, 19 Mar 2012 14:00:37 +0000 Subject: QPID-3895: Remove blocked channel/session from the list of blocked channels on channel/session close This patch adds the fllowing: - fixes AMQChannel to stop sending flow commands if channel is closing - fixes AMQChannel#compareTo ServerSession#compareTo - removes AMQSessionModel#getID() method from AMQChannel and Server session in order to avoid confusions Applied patch from Oleksandr Rudyy . git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1302455 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/qpid/server/AMQChannel.java | 9 +-- .../qpid/server/protocol/AMQProtocolEngine.java | 5 +- .../qpid/server/protocol/AMQSessionModel.java | 4 +- .../qpid/server/transport/ServerSession.java | 9 +-- .../org/apache/qpid/server/AMQChannelTest.java | 52 +++++++++++++++++ .../qpid/server/transport/ServerSessionTest.java | 67 ++++++++++++++++++++++ 6 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java create mode 100644 java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java (limited to 'java/broker') diff --git a/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java b/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java index 588b2079f2..22f9544b0c 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java +++ b/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java @@ -1123,11 +1123,6 @@ public class AMQChannel implements SessionConfig, AMQSessionModel, AsyncAutoComm } - public Object getID() - { - return _channelId; - } - public AMQConnectionModel getConnectionModel() { return _session; @@ -1377,7 +1372,7 @@ public class AMQChannel implements SessionConfig, AMQSessionModel, AsyncAutoComm { if(_blockingQueues.remove(queue)) { - if(_blocking.compareAndSet(true,false)) + if(_blocking.compareAndSet(true,false) && !isClosing()) { _actor.message(_logSubject, ChannelMessages.FLOW_REMOVED()); @@ -1627,6 +1622,6 @@ public class AMQChannel implements SessionConfig, AMQSessionModel, AsyncAutoComm public int compareTo(AMQSessionModel session) { - return getId().toString().compareTo(session.getID().toString()); + return getId().compareTo(session.getId()); } } diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java index f6980be525..1a055240b9 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java +++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java @@ -1315,7 +1315,8 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr public void closeSession(AMQSessionModel session, AMQConstant cause, String message) throws AMQException { - closeChannel((Integer)session.getID()); + int channelId = ((AMQChannel)session).getChannelId(); + closeChannel(channelId); MethodRegistry methodRegistry = getMethodRegistry(); ChannelCloseBody responseBody = @@ -1324,7 +1325,7 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr new AMQShortString(message), 0,0); - writeFrame(responseBody.generateFrame((Integer)session.getID())); + writeFrame(responseBody.generateFrame(channelId)); } public void close(AMQConstant cause, String message) throws AMQException diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java index a69f2a74ee..fa171815ca 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java +++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java @@ -20,6 +20,7 @@ */ package org.apache.qpid.server.protocol; +import java.util.UUID; import java.util.concurrent.ConcurrentSkipListSet; import org.apache.qpid.AMQException; @@ -35,7 +36,8 @@ import org.apache.qpid.server.queue.SimpleAMQQueue; */ public interface AMQSessionModel extends Comparable { - public Object getID(); + /** Unique session ID across entire broker*/ + public UUID getId(); public AMQConnectionModel getConnectionModel(); diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java index 9f7b8c53b8..462e880e5f 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java +++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java @@ -720,11 +720,6 @@ public class ServerSession extends Session close(); } - public Object getID() - { - return getName(); - } - public AMQConnectionModel getConnectionModel() { return getConnection(); @@ -854,7 +849,6 @@ public class ServerSession extends Session // unregister subscriptions in order to prevent sending of new messages // to subscriptions with closing session unregisterSubscriptions(); - super.close(); } @@ -1025,6 +1019,7 @@ public class ServerSession extends Session public int compareTo(AMQSessionModel session) { - return getId().toString().compareTo(session.getID().toString()); + return getId().compareTo(session.getId()); } + } diff --git a/java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java b/java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java new file mode 100644 index 0000000000..fc6cbcb248 --- /dev/null +++ b/java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java @@ -0,0 +1,52 @@ +/* + * + * 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.qpid.server; + +import org.apache.qpid.server.protocol.AMQProtocolSession; +import org.apache.qpid.server.protocol.InternalTestProtocolSession; +import org.apache.qpid.server.registry.ApplicationRegistry; +import org.apache.qpid.server.util.InternalBrokerBaseCase; +import org.apache.qpid.server.virtualhost.VirtualHost; + +public class AMQChannelTest extends InternalBrokerBaseCase +{ + private VirtualHost _virtualHost; + private AMQProtocolSession _protocolSession; + + @Override + public void setUp() throws Exception + { + super.setUp(); + _virtualHost = ApplicationRegistry.getInstance().getVirtualHostRegistry().getVirtualHosts().iterator().next(); + _protocolSession = new InternalTestProtocolSession(_virtualHost); + } + + public void testCompareTo() throws Exception + { + AMQChannel channel1 = new AMQChannel(_protocolSession, 1, _virtualHost.getMessageStore()); + + // create a channel with the same channelId but on a different session + AMQChannel channel2 = new AMQChannel(new InternalTestProtocolSession(_virtualHost), 1, _virtualHost.getMessageStore()); + assertFalse("Unexpected compare result", channel1.compareTo(channel2) == 0); + assertEquals("Unexpected compare result", 0, channel1.compareTo(channel1)); + } + +} diff --git a/java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java b/java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java new file mode 100644 index 0000000000..d775b0f2f8 --- /dev/null +++ b/java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java @@ -0,0 +1,67 @@ +/* + * + * 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.qpid.server.transport; + +import java.util.UUID; + +import org.apache.qpid.server.configuration.MockConnectionConfig; +import org.apache.qpid.server.registry.ApplicationRegistry; +import org.apache.qpid.server.util.InternalBrokerBaseCase; +import org.apache.qpid.server.virtualhost.VirtualHost; +import org.apache.qpid.transport.Binary; + +public class ServerSessionTest extends InternalBrokerBaseCase +{ + + private VirtualHost _virtualHost; + + @Override + public void setUp() throws Exception + { + super.setUp(); + _virtualHost = ApplicationRegistry.getInstance().getVirtualHostRegistry().getVirtualHosts().iterator().next(); + } + + public void testCompareTo() throws Exception + { + ServerConnection connection = new ServerConnection(1); + connection.setConnectionConfig(createConnectionConfig()); + ServerSession session1 = new ServerSession(connection, new ServerSessionDelegate(), + new Binary(getName().getBytes()), 0 , connection.getConfig()); + + // create a session with the same name but on a different connection + ServerConnection connection2 = new ServerConnection(2); + connection2.setConnectionConfig(createConnectionConfig()); + ServerSession session2 = new ServerSession(connection2, new ServerSessionDelegate(), + new Binary(getName().getBytes()), 0 , connection2.getConfig()); + + assertFalse("Unexpected compare result", session1.compareTo(session2) == 0); + assertEquals("Unexpected compare result", 0, session1.compareTo(session1)); + } + + private MockConnectionConfig createConnectionConfig() + { + return new MockConnectionConfig(UUID.randomUUID(), null, null, + false, 1, _virtualHost, "address", Boolean.TRUE, Boolean.TRUE, Boolean.TRUE, + "authid", "remoteProcessName", new Integer(1967), new Integer(1970), _virtualHost.getConfigStore(), Boolean.FALSE); + } + +} -- cgit v1.2.1