Repository: qpid-jms Updated Branches: refs/heads/master fa7445ffb -> 75f3cf8d2
QPIDJMS-123 Add additional protections from possible NPE on concurrent access. Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/75f3cf8d Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/75f3cf8d Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/75f3cf8d Branch: refs/heads/master Commit: 75f3cf8d2b94f495af6c130856ae8a9904ce670b Parents: fa7445f Author: Timothy Bish <[email protected]> Authored: Mon Oct 12 16:11:41 2015 -0400 Committer: Timothy Bish <[email protected]> Committed: Mon Oct 12 16:11:41 2015 -0400 ---------------------------------------------------------------------- .../org/apache/qpid/jms/JmsQueueBrowser.java | 78 ++++++++++---------- 1 file changed, 38 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/75f3cf8d/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsQueueBrowser.java ---------------------------------------------------------------------- diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsQueueBrowser.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsQueueBrowser.java index 1bb1ac6..af1096b 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsQueueBrowser.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsQueueBrowser.java @@ -23,6 +23,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import javax.jms.IllegalStateException; import javax.jms.JMSException; import javax.jms.Message; +import javax.jms.MessageConsumer; import javax.jms.Queue; import javax.jms.QueueBrowser; @@ -61,7 +62,7 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { private final JmsDestination destination; private final String selector; - private JmsMessageConsumer consumer; + private volatile JmsMessageConsumer consumer; private Message next; private final AtomicBoolean closed = new AtomicBoolean(); @@ -84,18 +85,6 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { this.selector = selector; } - private void destroyConsumer() { - if (consumer == null) { - return; - } - try { - consumer.close(); - consumer = null; - } catch (JMSException e) { - e.printStackTrace(); - } - } - /** * Gets an enumeration for browsing the current queue messages in the order they would be * received. @@ -108,16 +97,9 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { @Override public Enumeration<Message> getEnumeration() throws JMSException { checkClosed(); - if (consumer == null) { - consumer = createConsumer(); - } - return this; - } + createConsumer(); - private void checkClosed() throws IllegalStateException { - if (closed.get()) { - throw new IllegalStateException("The Consumer is closed"); - } + return this; } /** @@ -126,10 +108,9 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { @Override public boolean hasMoreElements() { while (true) { - synchronized (this) { - if (consumer == null) { - return false; - } + MessageConsumer consumer = this.consumer; + if (consumer == null) { + return false; } if (next == null) { @@ -160,11 +141,6 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { */ @Override public Message nextElement() { - synchronized (this) { - if (consumer == null) { - return null; - } - } if (hasMoreElements()) { Message message = next; @@ -195,7 +171,6 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { * if the JMS provider fails to get the queue associated with this browser due to * some internal error. */ - @Override public Queue getQueue() throws JMSException { return (Queue) destination; @@ -212,15 +187,38 @@ public class JmsQueueBrowser implements QueueBrowser, Enumeration<Message> { return "JmsQueueBrowser { value=" + (consumer != null ? consumer.getConsumerId() : "null") + " }"; } - private JmsMessageConsumer createConsumer() throws JMSException { - JmsMessageConsumer rc = new JmsMessageConsumer(session.getNextConsumerId(), session, destination, selector, false) { + private void checkClosed() throws IllegalStateException { + if (closed.get()) { + throw new IllegalStateException("The Consumer is closed"); + } + } - @Override - public boolean isBrowser() { - return true; + private synchronized void destroyConsumer() { + synchronized (this) { + try { + if (consumer != null) { + consumer.close(); + consumer = null; + } + } catch (JMSException e) { + LOG.warn("Error closing down internal consumer: ", e); } - }; - rc.init(); - return rc; + } + } + + private synchronized void createConsumer() throws JMSException { + if (consumer == null) { + JmsMessageConsumer result = new JmsMessageConsumer(session.getNextConsumerId(), session, destination, selector, false) { + + @Override + public boolean isBrowser() { + return true; + } + }; + result.init(); + + // Assign only after fully created and initialized. + consumer = result; + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
