Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE #1854

Closed
wants to merge 1 commit into from
Closed

ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE #1854

wants to merge 1 commit into from

Conversation

arshadmohammad
Copy link
Contributor

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

@arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
15b9763 
Moved channel object null check to sendPkt method to cover all calling scenarios
sonatype-lift[bot]
sonatype-lift bot reviewed Apr 7, 2022
View reviewed changes
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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 with help or ignore)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sonatype-lift ignore

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no new code added which is related to THREAD_SAFETY_VIOLATION

symat
symat approved these changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@symat symat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!
We should push this back to all active branches (if it is an easy cherry-pick).

@arshadmohammad
Copy link
Contributor Author

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

@asfgit asfgit closed this in d5876e8 Apr 8, 2022
asfgit pushed a commit that referenced this pull request Apr 8, 2022
@arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
f28d167 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes #1854 from arshadmohammad/ZOOKEEPER-4514-npe

(cherry picked from commit d5876e8)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
asfgit pushed a commit that referenced this pull request Apr 8, 2022
@arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
fab8899 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes #1854 from arshadmohammad/ZOOKEEPER-4514-npe

(cherry picked from commit d5876e8)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
asfgit pushed a commit that referenced this pull request Apr 8, 2022
@arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
cb79fb9 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes #1854 from arshadmohammad/ZOOKEEPER-4514-npe

(cherry picked from commit d5876e8)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
asfgit pushed a commit that referenced this pull request May 17, 2022
@arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
d3fd534 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes #1854 from arshadmohammad/ZOOKEEPER-4514-npe

(cherry picked from commit d5876e8)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
(cherry picked from commit cb79fb9)
anuragmadnawat1 pushed a commit to anuragmadnawat1/zookeeper that referenced this pull request Nov 1, 2022
@arshadmohammad @anurag-harness
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
5e0efa4 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes apache#1854 from arshadmohammad/ZOOKEEPER-4514-npe
@anuragmadnawat1 anuragmadnawat1 mentioned this pull request Nov 1, 2022
Merged
anuragmadnawat1 added a commit to anuragmadnawat1/zookeeper that referenced this pull request Nov 1, 2022
@anuragmadnawat1 @arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE (#54)
7237287 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes apache#1854 from arshadmohammad/ZOOKEEPER-4514-npe

Co-authored-by: Mohammad Arshad <arshad@apache.org>
anurag-harness pushed a commit to anurag-harness/zookeeper that referenced this pull request Jan 13, 2023
@arshadmohammad @anurag-harness
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
76b48b8 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes apache#1854 from arshadmohammad/ZOOKEEPER-4514-npe
@anurag-harness anurag-harness mentioned this pull request Jan 13, 2023
Merged
anurag-harness added a commit to anurag-harness/zookeeper that referenced this pull request Jan 13, 2023
@anurag-harness @arshadmohammad
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE (#21)
3b5b7b2 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes apache#1854 from arshadmohammad/ZOOKEEPER-4514-npe

Co-authored-by: Mohammad Arshad <arshad@apache.org>
desaikomal pushed a commit to linkedin/zookeeper that referenced this pull request Jun 17, 2023
@arshadmohammad @desaikomal
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
8f3127c 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes apache#1854 from arshadmohammad/ZOOKEEPER-4514-npe

(cherry picked from commit d5876e8)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
desaikomal pushed a commit to linkedin/zookeeper that referenced this pull request Jun 27, 2023
@arshadmohammad @desaikomal
ZOOKEEPER-4514: ClientCnxnSocketNetty throwing NPE
fa3a72c 
Moved channel object null check to sendPkt method to cover all calling scenarios

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>

Closes apache#1854 from arshadmohammad/ZOOKEEPER-4514-npe

(cherry picked from commit d5876e8)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonatype-lift sonatype-lift[bot] sonatype-lift left review comments

@symat symat symat approved these changes

@eolivelli eolivelli Awaiting requested review from eolivelli

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@arshadmohammad @symat