You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by pgfox <gi...@git.apache.org> on 2017/11/28 08:00:25 UTC
[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...
GitHub user pgfox opened a pull request:
https://github.com/apache/activemq-artemis/pull/1675
ARTEMIS-1526 NullpointerException in ActiveMQServerControl.listConsumers()
race condition between listConsumers() and client closing a Session . Avoid NullPointerException and return empty strings for JSON attributes associated with that Session.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pgfox/activemq-artemis listConsumer_race
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1675.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 #1675
----
commit 51cea804aaf79372eb88e3541a2b1b2bb0b7f594
Author: Pat Fox <pa...@gmail.com>
Date: 2017-11-26T14:00:23Z
ARTEMIS-1526 race condition between listConsumers() and closing a Session. Avoid NullPointerException and return empty strings for Json attributes associated with that Session.
----
---
[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...
Posted by pgfox <gi...@git.apache.org>.
Github user pgfox commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1675#discussion_r153589625
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/view/ConsumerView.java ---
@@ -45,7 +45,7 @@ public Class getClassT() {
@Override
public JsonObjectBuilder toJson(ServerConsumer consumer) {
ServerSession session = server.getSessionByID(consumer.getSessionID());
- JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", toString(session.getName())).add("clientID", toString(session.getRemotingConnection().getClientID())).add("user", toString(session.getUsername())).add("protocol", toString(session.getRemotingConnection().getProtocolName())).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", toString(session.getRemotingConnection().getTransportConnection().getLocalAddress())).add("remoteAddress", toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress())).add("creationTime", new Date(consumer.getCreationTime()).toString());
+ JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", (session == null ? "" : toString(session.getName()))).add("clientID", (session == null ? "" : toString(session.getRemotingConnection().getClientID()))).add("user", (session == null ? "" : toString(session.getUsername()))).add("protocol", (session == null ? "" : toString(session.getRemotingConnection().getProtocolName()))).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getLocalAddress()))).add("remoteAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress()))).add("creationTime", new Date(consumer.getCreationTime()).toString());
--- End diff --
@clebertsuconic originally my thought was to get as much info as possible for the consumer but your suggestion makes more sense as that consumer will also be closed, so there is no real point in displaying the available data.
I will update/test and push it again
Thanks
Pat
---
[GitHub] activemq-artemis issue #1675: ARTEMIS-1526 NullpointerException in ActiveMQS...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1675
@pgfox you need to rebase this...
---
[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1675#discussion_r153589884
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/view/ConsumerView.java ---
@@ -45,7 +45,7 @@ public Class getClassT() {
@Override
public JsonObjectBuilder toJson(ServerConsumer consumer) {
ServerSession session = server.getSessionByID(consumer.getSessionID());
- JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", toString(session.getName())).add("clientID", toString(session.getRemotingConnection().getClientID())).add("user", toString(session.getUsername())).add("protocol", toString(session.getRemotingConnection().getProtocolName())).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", toString(session.getRemotingConnection().getTransportConnection().getLocalAddress())).add("remoteAddress", toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress())).add("creationTime", new Date(consumer.getCreationTime()).toString());
+ JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", (session == null ? "" : toString(session.getName()))).add("clientID", (session == null ? "" : toString(session.getRemotingConnection().getClientID()))).add("user", (session == null ? "" : toString(session.getUsername()))).add("protocol", (session == null ? "" : toString(session.getRemotingConnection().getProtocolName()))).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getLocalAddress()))).add("remoteAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress()))).add("creationTime", new Date(consumer.getCreationTime()).toString());
--- End diff --
@pgfox as you're on it.. if you could break this line into a few lines please?
---
[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1675#discussion_r153566039
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/view/ConsumerView.java ---
@@ -45,7 +45,7 @@ public Class getClassT() {
@Override
public JsonObjectBuilder toJson(ServerConsumer consumer) {
ServerSession session = server.getSessionByID(consumer.getSessionID());
- JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", toString(session.getName())).add("clientID", toString(session.getRemotingConnection().getClientID())).add("user", toString(session.getUsername())).add("protocol", toString(session.getRemotingConnection().getProtocolName())).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", toString(session.getRemotingConnection().getTransportConnection().getLocalAddress())).add("remoteAddress", toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress())).add("creationTime", new Date(consumer.getCreationTime()).toString());
+ JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", (session == null ? "" : toString(session.getName()))).add("clientID", (session == null ? "" : toString(session.getRemotingConnection().getClientID()))).add("user", (session == null ? "" : toString(session.getUsername()))).add("protocol", (session == null ? "" : toString(session.getRemotingConnection().getProtocolName()))).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getLocalAddress()))).add("remoteAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress()))).add("creationTime", new Date(consumer.getCreationTime()).toString());
--- End diff --
Instead of protecting every single attribute with session == null? why not simply do
if (session == null) continue?
That's one big line... getting even bigger.
---
[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...
Posted by pgfox <gi...@git.apache.org>.
Github user pgfox commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1675#discussion_r153590458
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/view/ConsumerView.java ---
@@ -45,7 +45,7 @@ public Class getClassT() {
@Override
public JsonObjectBuilder toJson(ServerConsumer consumer) {
ServerSession session = server.getSessionByID(consumer.getSessionID());
- JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", toString(session.getName())).add("clientID", toString(session.getRemotingConnection().getClientID())).add("user", toString(session.getUsername())).add("protocol", toString(session.getRemotingConnection().getProtocolName())).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", toString(session.getRemotingConnection().getTransportConnection().getLocalAddress())).add("remoteAddress", toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress())).add("creationTime", new Date(consumer.getCreationTime()).toString());
+ JsonObjectBuilder obj = JsonLoader.createObjectBuilder().add("id", toString(consumer.sequentialID())).add("session", (session == null ? "" : toString(session.getName()))).add("clientID", (session == null ? "" : toString(session.getRemotingConnection().getClientID()))).add("user", (session == null ? "" : toString(session.getUsername()))).add("protocol", (session == null ? "" : toString(session.getRemotingConnection().getProtocolName()))).add("queue", toString(consumer.getQueue().getName())).add("queueType", toString(consumer.getQueue().getRoutingType()).toLowerCase()).add("address", toString(consumer.getQueue().getAddress().toString())).add("localAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getLocalAddress()))).add("remoteAddress", (session == null ? "" : toString(session.getRemotingConnection().getTransportConnection().getRemoteAddress()))).add("creationTime", new Date(consumer.getCreationTime()).toString());
--- End diff --
@clebertsuconic sure will do
---
[GitHub] activemq-artemis issue #1675: ARTEMIS-1526 NullpointerException in ActiveMQS...
Posted by pgfox <gi...@git.apache.org>.
Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1675
seems there were changed introduced yesterday - I will need to pick these up retest & push again.
---
[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1675
---