[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322638#comment-16322638
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1772
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322637#comment-16322637
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1772
I'll buy that.
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322636#comment-16322636
]
ASF subversion and git services commented on ARTEMIS-1601:
--
Commit b8f591b6b9b84cbea599db95c6b743a2c4088319 in activemq-artemis's branch
refs/heads/master from jostbg
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=b8f591b ]
ARTEMIS-1601 deal with race in Stomp sendMessage
If the Stomp consumer was closed at or near the same time a message was
dispatched then an NPE might result. Throwing an exception is a
relatively expensive operation in the JVM because of the stacktrace
information that needs to be generated, so in cases where it is known
that a null value could be returned one should check and handle it
appropriately.
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322565#comment-16322565
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user jostbg commented on the issue:
https://github.com/apache/activemq-artemis/pull/1772
I wonder if that may be a race condition between StompSession#disconnect
and StompSession#sendMessage, i.e. if a disconnect happens while sendMessage is
being executed or something.
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322550#comment-16322550
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user jostbg commented on the issue:
https://github.com/apache/activemq-artemis/pull/1772
The NPEs occur that's why I opened this PR. Since they are only logged in
debug level I only saw them while debugging some stomp issue I had.
Throwing an exception is a relatively expensive operation in the JVM
because of the stacktrace information that needs to be generated, so in cases
where it is know that a null value could be returned it one should not rely on
an NPE to be catched.
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322512#comment-16322512
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1772
IMO, it's less clean because now there are 2 places which are returning 0
in case of an error. In any case, it's really not a big deal.
One additional question here...Is this NPE mainly theoretical or is there a
specific use-case which triggers it? If the former then I'm even less in favor
of merging the PR. If the latter then it would be worth adding a test or maybe
just describing the use-case in the commit message.
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322499#comment-16322499
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1772
+1 I think It’s cleaner than relying exception to occur
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322454#comment-16322454
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1772
I'm not sure this is actually necessary. `java.lang.NullPointerException`
extends `java.lang.RuntimeException` which in turn extends
`java.lang.Exception` which is already being caught for this `try` block and
returning 0.
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>Priority: Minor
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (ARTEMIS-1601) NPE silently thrown in StompSession#sendMessage
[
https://issues.apache.org/jira/browse/ARTEMIS-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322062#comment-16322062
]
ASF GitHub Bot commented on ARTEMIS-1601:
-
GitHub user jostbg opened a pull request:
https://github.com/apache/activemq-artemis/pull/1772
ARTEMIS-1601 Prevent NPE in StompSession#sendMessage
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jostbg/activemq-artemis ARTEMIS-1601
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1772.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1772
commit 185983d4dff8e9d256fc65cb2e1fe0e7b12ec8fb
Author: jostbg <35264802+jostbg@...>
Date: 2018-01-11T11:16:35Z
ARTEMIS-1601 Prevent NPE in StompSession#sendMessage
> NPE silently thrown in StompSession#sendMessage
> ---
>
> Key: ARTEMIS-1601
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1601
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
>Affects Versions: 2.4.0
> Environment: Artemis 2.5.0-SNAPSHOT
>Reporter: Johan Stenberg
>Assignee: Justin Bertram
>
> During debugging a stomp session I realized that the StompSession's send
> method does not check if looking up a subscription returns null:
> {code:java}
> StompSubscription subscription = subscriptions.get(consumer.getID());
> {code}
> This later sometimes results NPEs at:
> {code:java}
> if (subscription.getAck().equals(Stomp.Headers.Subscribe.AckModeValues.AUTO))
> {
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
