ZOOKEEPER-3009: Potential NPE in NIOServerCnxnFactory We have developed a static analysis tool [NPEDetector](https://github.com/lujiefsi/NPEDetector) to find some potential NPE. Our analysis shows that NPE reason can be simple:some callees may return null directly in corner case(e.g. node crash , IO exception), some of their callers have !=null check but some do not have. ### Bug: Callee getSocketAddress can return null, may caused by node crash, network exception....

    public InetAddress getSocketAddress() {
        if (sock.isOpen() == false) {
            return null;
        }
        return sock.socket().getInetAddress();
    }

getSocketAddress has two callers: removeCnxn and removeCnxn caller removeCnxn has null check

 public boolean removeCnxn(NIOServerCnxn cnxn) {
        //other code
        InetAddress addr = cnxn.getSocketAddress();
        if (addr != null) {
            Set set = ipMap.get(addr);
            //other code
        }
     //other code
    }

but caller addCnxn has the similar code, but don't have the null check:

     private void addCnxn(NIOServerCnxn cnxn) {
         InetAddress addr = cnxn.getSocketAddress();
         Set set = ipMap.get(addr);
        //other code
      }

I commit a patch to fix it: adding an null checker in addCnxn, and throws a semantics IOException rather than the ugly NPE. I think the IOException is ok, because the caller of addCnxn has the handler code:

  private void processAcceptedConnections() {
               //other code
               try {
                    addCnxn(cnxn);
               } catch (IOException e) {
                    // register, createConnection
                    cleanupSelectionKey(key);
                    fastCloseSock(accepted);
               }
             }

Maybe i am wrong, please point out my error. Author: LJ1043041006 <1239497420@qq.com> Reviewers: Patrick Hunt , Allan Lyu , Michael Han Closes #525 from lujiefsi/ZOOKEEPER-3009

PerChannelBookieClient is full of code duplication #525

Description

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions