You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2022/04/07 10:17:29 UTC

[GitHub] [zookeeper] arshadmohammad opened a new pull request, #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

arshadmohammad opened a new pull request, #1854:
URL: https://github.com/apache/zookeeper/pull/1854

   Moved channel object null check to sendPkt method to cover all calling scenarios


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] arshadmohammad commented on pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on PR #1854:
URL: https://github.com/apache/zookeeper/pull/1854#issuecomment-1092735104

   Thanks @symat for the review. I will merge it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] sonatype-lift[bot] commented on a diff in pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1854:
URL: https://github.com/apache/zookeeper/pull/1854#discussion_r844976406


##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java:
##########
@@ -374,9 +377,6 @@ private void doWrite(Queue<Packet> pendingQueue, Packet p, ClientCnxn cnxn) {
 
     @Override
     void sendPacket(ClientCnxn.Packet p) throws IOException {
-        if (channel == null) {
-            throw new IOException("channel has been closed");
-        }
         sendPktAndFlush(p);

Review Comment:
   *THREAD_SAFETY_VIOLATION:*  Read/Write race. Non-private method `ClientCnxnSocketNetty.sendPacket(...)` indirectly reads without synchronization from `this.channel`. Potentially races with write in method `ClientCnxnSocketNetty.cleanup()`.
    Reporting because another access to the same memory occurs on a background thread, although this access may not.
   
   (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] sonatype-lift[bot] commented on a diff in pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1854:
URL: https://github.com/apache/zookeeper/pull/1854#discussion_r845019001


##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java:
##########
@@ -374,9 +377,6 @@ private void doWrite(Queue<Packet> pendingQueue, Packet p, ClientCnxn cnxn) {
 
     @Override
     void sendPacket(ClientCnxn.Packet p) throws IOException {
-        if (channel == null) {
-            throw new IOException("channel has been closed");
-        }
         sendPktAndFlush(p);

Review Comment:
   I've recorded this as ignored for this pull request. If you change your mind, just comment `@sonatype-lift unignore`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] asfgit closed pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
URL: https://github.com/apache/zookeeper/pull/1854


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] arshadmohammad commented on a diff in pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on code in PR #1854:
URL: https://github.com/apache/zookeeper/pull/1854#discussion_r845019231


##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java:
##########
@@ -374,9 +377,6 @@ private void doWrite(Queue<Packet> pendingQueue, Packet p, ClientCnxn cnxn) {
 
     @Override
     void sendPacket(ClientCnxn.Packet p) throws IOException {
-        if (channel == null) {
-            throw new IOException("channel has been closed");
-        }
         sendPktAndFlush(p);

Review Comment:
   There is no new code added which is related to THREAD_SAFETY_VIOLATION



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] arshadmohammad commented on a diff in pull request #1854: ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE

Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on code in PR #1854:
URL: https://github.com/apache/zookeeper/pull/1854#discussion_r845018961


##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java:
##########
@@ -374,9 +377,6 @@ private void doWrite(Queue<Packet> pendingQueue, Packet p, ClientCnxn cnxn) {
 
     @Override
     void sendPacket(ClientCnxn.Packet p) throws IOException {
-        if (channel == null) {
-            throw new IOException("channel has been closed");
-        }
         sendPktAndFlush(p);

Review Comment:
   @sonatype-lift ignore



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org