diff options
Diffstat (limited to 'qpid/java')
2 files changed, 38 insertions, 2 deletions
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionList.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionList.java index 6dabfd21e2..3e6299cb8a 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionList.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionList.java @@ -191,7 +191,7 @@ public class SubscriptionList return false; } - private void nodeMarkerCleanup(SubscriptionNode node) + private void nodeMarkerCleanup(final SubscriptionNode node) { SubscriptionNode markedNode = _subNodeMarker.get(); if(node == markedNode) @@ -200,8 +200,10 @@ public class SubscriptionList //replace it with a dummy pointing at the next node. //this is OK as the marked node is only used to index //into the list and find the next node to use. + //Because we inserted a dummy if node was the + //tail, markedNode.nextNode() can never be null. SubscriptionNode dummy = new SubscriptionNode(); - dummy.setNext(markedNode.findNext()); + dummy.setNext(markedNode.nextNode()); //if the CAS fails the marked node has changed, thus //we don't care about the dummy and just forget it diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/subscription/SubscriptionListTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/subscription/SubscriptionListTest.java index a9acfccd8e..c4d1a1e614 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/subscription/SubscriptionListTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/subscription/SubscriptionListTest.java @@ -303,6 +303,40 @@ public class SubscriptionListTest extends QpidTestCase } /** + * Test that the marked node 'findNext' behaviour is as expected after a subscription is added + * to the list following the tail subscription node being removed while it is the marked node. + * That is, that the new subscriptions node is returned by getMarkedNode().findNext(). + */ + public void testMarkedNodeFindsNewSubscriptionAfterRemovingTailWhilstMarked() + { + //get the node out the list for the 3rd subscription + SubscriptionNode sub3Node = getNodeForSubscription(_subList, _sub3); + assertNotNull("Should have been a node present for the subscription", sub3Node); + + //mark the 3rd subscription node + assertTrue("should have succeeded in updating the marked node", + _subList.updateMarkedNode(_subList.getMarkedNode(), sub3Node)); + + //verify calling findNext on the marked node returns null, i.e. the end of the list has been reached + assertEquals("Unexpected node after marked node", null, _subList.getMarkedNode().findNext()); + + //remove the 3rd(marked) subscription from the list + assertTrue("Removing subscription node should have succeeded", _subList.remove(_sub3)); + + //add a new 4th subscription to the list + Subscription sub4 = new MockSubscription(); + _subList.add(sub4); + + //get the node out the list for the 4th subscription + SubscriptionNode sub4Node = getNodeForSubscription(_subList, sub4); + assertNotNull("Should have been a node present for the subscription", sub4Node); + + //verify the marked node (which is now a dummy substitute for the 3rd subscription) returns + //the 4th subscriptions node as the next non-deleted node. + assertEquals("Unexpected next node", sub4Node, _subList.getMarkedNode().findNext()); + } + + /** * Test that setting the marked node to null doesn't cause problems during remove operations */ public void testRemoveWithNullMarkedNode() |