ZOOKEEPER-3895: Client side NullPointerException in case of empty Mul…

…ti operation https://issues.apache.org/jira/browse/ZOOKEEPER-3895 Author: Enrico Olivelli <eolivelli@apache.org> Reviewers: tison <wander4096@gmail.com>, Mate Szalay-Beko <symat@apache.org> Closes apache#1407 from eolivelli/fix/ZOOKEEPER-3895
narendly · Jul 29, 2020 · ce523e0 · ce523e0
1 parent c9f1521
commit ce523e0
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 0 deletions.
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
@@ -30,6 +30,7 @@
 import java.nio.ByteBuffer;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -469,6 +470,16 @@ private static String makeThreadName(String suffix) {
         return name + suffix;
     }
 
+    /**
+     * Tests that current thread is the main event loop.
+     * This method is useful only for tests inside ZooKeeper project
+     * it is not a public API intended for use by external applications.
+     * @return true if Thread.currentThread() is an EventThread.
+     */
+    public static boolean isInEventThread() {
+        return Thread.currentThread() instanceof EventThread;
+    }
+
     class EventThread extends ZooKeeperThread {
 
         private final LinkedBlockingQueue<Object> waitingEvents = new LinkedBlockingQueue<Object>();
@@ -589,6 +600,8 @@ private void processEvent(Object event) {
                         ((AsyncCallback.EphemeralsCallback) lcb.cb).processResult(lcb.rc, lcb.ctx, null);
                     } else if (lcb.cb instanceof AsyncCallback.AllChildrenNumberCallback) {
                         ((AsyncCallback.AllChildrenNumberCallback) lcb.cb).processResult(lcb.rc, lcb.path, lcb.ctx, -1);
+                    } else if (lcb.cb instanceof AsyncCallback.MultiCallback) {
+                        ((AsyncCallback.MultiCallback) lcb.cb).processResult(lcb.rc, lcb.path, lcb.ctx, Collections.emptyList());
                     } else {
                         ((VoidCallback) lcb.cb).processResult(lcb.rc, lcb.path, lcb.ctx);
                     }

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
@@ -24,6 +24,7 @@
 import java.net.SocketAddress;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -1711,6 +1712,11 @@ protected void multiInternal(
         MultiOperationRecord request,
         MultiCallback cb,
         Object ctx) throws IllegalArgumentException {
+        if (request.size() == 0) {
+            // nothing to do, early exit
+            cnxn.queueCallback(cb, KeeperException.Code.OK.intValue(), null, ctx);
+            return;
+        }
         RequestHeader h = new RequestHeader();
         switch (request.getOpKind()) {
         case TRANSACTION:
@@ -1729,6 +1735,10 @@ protected void multiInternal(
     protected List<OpResult> multiInternal(
         MultiOperationRecord request) throws InterruptedException, KeeperException, IllegalArgumentException {
         RequestHeader h = new RequestHeader();
+        if (request.size() == 0) {
+            // nothing to do, early exit
+            return Collections.emptyList();
+        }
         switch (request.getOpKind()) {
         case TRANSACTION:
             h.setType(ZooDefs.OpCode.multi);

diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/MultiOperationTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/MultiOperationTest.java
@@ -39,6 +39,7 @@
 import java.util.stream.Collectors;
 import org.apache.zookeeper.AsyncCallback;
 import org.apache.zookeeper.AsyncCallback.MultiCallback;
+import org.apache.zookeeper.ClientCnxn;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.Op;
@@ -107,6 +108,9 @@ private List<OpResult> multi(ZooKeeper zk, Iterable<Op> ops) throws KeeperExcept
             zk.multi(ops, new MultiCallback() {
                 @Override
                 public void processResult(int rc, String path, Object ctx, List<OpResult> opResults) {
+                    if (!ClientCnxn.isInEventThread()) {
+                        throw new RuntimeException("not in event thread");
+                    }
                     synchronized (res) {
                         res.rc = rc;
                         res.results = opResults;
@@ -442,6 +446,11 @@ public void testCreate() throws Exception {
         zk.getData("/multi2", false, null);
     }
 
+    @Test
+    public void testEmpty() throws Exception {
+        multi(zk, Arrays.asList());
+    }
+
     @Test
     public void testCreateDelete() throws Exception {