Skip to content

Commit

Permalink
ZOOKEEPER-1875: NullPointerException in ClientCnxn$EventThread.proces…
Browse files Browse the repository at this point in the history
…sEvent

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: Mate Szalay-Beko <symat@apache.org>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1855 from arshadmohammad/ZOOKEEPER-1875-npe and squashes the following commits:

4a7d471 [Mohammad Arshad] Corrected impacted test cases
12f44d6 [Mohammad Arshad] ZOOKEEPER-1875: NullPointerException in ClientCnxn$EventThread.processEvent

(cherry picked from commit 86690ff)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
  • Loading branch information
arshadmohammad authored and desaikomal committed Jun 17, 2023
1 parent d9b24dd commit 79a6e55
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 13 deletions.
78 changes: 67 additions & 11 deletions zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,12 @@ public boolean isConnected() {
* @throws IOException
* in cases of network failure
* @throws IllegalArgumentException
* if an invalid chroot path is specified
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) throws IOException {
this(connectString, sessionTimeout, watcher, false);
Expand Down Expand Up @@ -863,7 +868,12 @@ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) thro
* @throws IOException
* in cases of network failure
* @throws IllegalArgumentException
* if an invalid chroot path is specified
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -928,7 +938,12 @@ public ZooKeeper(
* @throws IOException
* in cases of network failure
* @throws IllegalArgumentException
* if an invalid chroot path is specified
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -996,7 +1011,12 @@ public ZooKeeper(
* @throws IOException
* in cases of network failure
* @throws IllegalArgumentException
* if an invalid chroot path is specified
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand All @@ -1011,6 +1031,7 @@ public ZooKeeper(
sessionTimeout,
watcher);

validateWatcher(watcher);
if (clientConfig == null) {
clientConfig = new ZKClientConfig();
}
Expand Down Expand Up @@ -1100,7 +1121,12 @@ protected ClientCnxn createConnection(
* @throws IOException
* in cases of network failure
* @throws IllegalArgumentException
* if an invalid chroot path is specified
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -1162,7 +1188,12 @@ public ZooKeeper(
* @throws IOException
* in cases of network failure
* @throws IllegalArgumentException
* if an invalid chroot path is specified
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -1228,8 +1259,13 @@ public ZooKeeper(
* password for this session
*
* @throws IOException in cases of network failure
* @throws IllegalArgumentException if an invalid chroot path is specified
* @throws IllegalArgumentException for an invalid list of ZooKeeper hosts
* @throws IllegalArgumentException
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -1302,7 +1338,13 @@ public ZooKeeper(
* @param aHostProvider
* use this as HostProvider to enable custom behaviour.
* @throws IOException in cases of network failure
* @throws IllegalArgumentException if an invalid chroot path is specified
* @throws IllegalArgumentException
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -1389,7 +1431,12 @@ public ZooKeeper(
* configuring properties differently compared to other instances
* @throws IOException in cases of network failure
* @throws IllegalArgumentException if an invalid chroot path is specified
*
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
* @since 3.5.5
*/
public ZooKeeper(
Expand All @@ -1410,6 +1457,7 @@ public ZooKeeper(
Long.toHexString(sessionId),
(sessionPasswd == null ? "<null>" : "<hidden>"));

validateWatcher(watcher);
if (clientConfig == null) {
clientConfig = new ZKClientConfig();
}
Expand Down Expand Up @@ -1493,7 +1541,13 @@ public ZooKeeper(
* allowed while write requests are not. It continues seeking for
* majority in the background.
* @throws IOException in cases of network failure
* @throws IllegalArgumentException if an invalid chroot path is specified
* @throws IllegalArgumentException
* if any of the following is true:
* <ul>
* <li> if an invalid chroot path is specified
* <li> for an invalid list of ZooKeeper hosts
* <li> watcher is null
* </ul>
*/
public ZooKeeper(
String connectString,
Expand Down Expand Up @@ -1581,10 +1635,12 @@ public void addAuthInfo(String scheme, byte[] auth) {
/**
* Specify the default watcher for the connection (overrides the one
* specified during construction).
* @throws IllegalArgumentException if watcher is null
*
* @param watcher
*/
public synchronized void register(Watcher watcher) {
validateWatcher(watcher);
watchManager.defaultWatcher = watcher;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,4 +726,28 @@ public void testWaitForConnection() throws Exception {
assertTrue("ZooKeeperMain does not wait until the specified timeout",
endTime - startTime >= timeout);
}

@Test
public void testInvalidWatcherRegistration() throws Exception {
String nullWatcherMsg = "Invalid Watcher, shouldn't be null!";
final ZooKeeper zk = createClient();
try {
zk.register(null);
fail("Should validate null watcher!");
} catch (IllegalArgumentException iae) {
assertEquals(nullWatcherMsg, iae.getMessage());
}
try {
new ZooKeeper(hostPort, 10000, null, false);
fail("Should validate null watcher!");
} catch (IllegalArgumentException iae) {
assertEquals(nullWatcherMsg, iae.getMessage());
}
try {
new ZooKeeper(hostPort, 10000, null, 10L, "".getBytes(), false);
fail("Should validate null watcher!");
} catch (IllegalArgumentException iae) {
assertEquals(nullWatcherMsg, iae.getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private void startServers(List<Integer> sids) throws InterruptedException {

private void triggerOps(int sid, String prefix) throws Exception {
TxnLogDigestTest.performOperations(servers.zk[sid], prefix);
servers.restartClient(sid, null);
servers.restartClient(sid, event -> { });
waitForOne(servers.zk[sid], States.CONNECTED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ public void testRevalidation() throws Exception {
public void testInOrderCommits() throws Exception {
setUp(-1);

zk = new ZooKeeper("127.0.0.1:" + CLIENT_PORT_QP1, ClientBase.CONNECTION_TIMEOUT, null);
zk = new ZooKeeper("127.0.0.1:" + CLIENT_PORT_QP1, ClientBase.CONNECTION_TIMEOUT, event -> { });
for (int i = 0; i < 10; i++) {
zk.create("/bulk"
+ i, ("Initial data of some size").getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
Expand Down

0 comments on commit 79a6e55

Please sign in to comment.