[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

pgfox
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 <[hidden email]>
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.

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

pgfox
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

pgfox
In reply to this post by pgfox
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



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

pgfox
In reply to this post by pgfox
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?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

pgfox
In reply to this post by pgfox
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


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1675: ARTEMIS-1526 NullpointerException in ActiveMQS...

pgfox
In reply to this post by pgfox
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1675: ARTEMIS-1526 NullpointerException in ActiveMQS...

pgfox
In reply to this post by pgfox
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1675
 
    @pgfox you need to rebase this...


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1675: ARTEMIS-1526 NullpointerException in Ac...

pgfox
In reply to this post by pgfox
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1675


---