symat commented on a change in pull request #1681:
URL: https://github.com/apache/zookeeper/pull/1681#discussion_r611652247
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java
##########
@@ -520,6 +520,9 @@ private void receiveMessage(ByteBuf message) {
if (len < 0 || len > BinaryInputArchive.maxBuffer) {
throw new IOException("Len error " + len);
}
+ if (zkServer == null || !zkServer.isRunning()) {
+ throw new IOException("ZK down");
+ }
Review comment:
actually I would prefer to use the following approach, copied from an
other part of this function:
https://github.com/apache/zookeeper/blob/7fdadf7273f34dd0552db25a3771cf55b65e9208/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java#L475-L478
This way we kind of "save" the reference of `zkServer` to an other variable
and if `zkServer` becomes null right after we did the check, we are still in a
safe spot. Although the final solution would be to use locks and not allowing
to use/modify `zkServer` paralelly from multiple threads... But that would be a
more serious refactoring.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
