Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARTEMIS-2189 allow deleting temp dest when session is closed #2448

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@jbertram
Copy link
Contributor

commented Dec 3, 2018

No description provided.

@@ -265,6 +266,126 @@ public void testTemporaryQueueDeleted() throws Exception {
}
}

@Test
public void testTemporaryQueueDeletedAfterSessionClosed() throws Exception {

This comment has been minimized.

Copy link
@michaelandrepearce

michaelandrepearce Dec 3, 2018

Contributor

Can the behaviour be checked with amqp and openwire to check impl behaviour is the same

This comment has been minimized.

Copy link
@tabish121

tabish121 Dec 3, 2018

Contributor

For AMQP the handling of dynamic nodes is tested here:
https://github.com/apache/activemq-artemis/blob/master/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpTempDestinationTest.java
And here:
https://github.com/apache/activemq-artemis/blob/master/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSTemporaryDestinationTest.java

For OpenWire you'd want to look for a test of the RemoveInfo command that carries a DestintionInfo object.

Other tests that are specifically targeted the JMS spec (or other) compliant behaviour of a client really belong to the project that maintains the client in question.

This comment has been minimized.

Copy link
@jbertram

jbertram Dec 3, 2018

Author Contributor

The issue here is not with the broker but with the core client. Therefore, I'm not sure it makes sense to test other clients on this particular use-case.

if (session.getCoreSession().isClosed()) {
// Temporary queues will be deleted when the connection is closed.. nothing to be done then!
return;
/**

This comment has been minimized.

Copy link
@tabish121

tabish121 Dec 4, 2018

Contributor

@jbertram This code is still a bit racey in that the session could become closed right after checking that it isn't and thereby still allow a violation in what the spec says should be possible. It might make more sense to just use the approach of creating a session for this operation each time given this isn't something people are going to be doing a great deal and so doesn't need to be highly performant.

This comment has been minimized.

Copy link
@jbertram

jbertram Dec 5, 2018

Author Contributor

Fair enough. I pushed an update.

This comment has been minimized.

Copy link
@franz1981

franz1981 Dec 6, 2018

Contributor

@tabish121 The updated version is fine mate? If is fine I will merge it 👍

This comment has been minimized.

Copy link
@tabish121

tabish121 Dec 6, 2018

Contributor

Looks ok to me now, shouldn't be racing with session close now.

@jbertram jbertram force-pushed the jbertram:ARTEMIS-2189 branch from ff41765 to 39bf16a Dec 5, 2018

@asfgit asfgit closed this in f5b509b Dec 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.