From commits-return-7662-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Wed Mar 6 02:34:02 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 7BED7180648 for ; Wed, 6 Mar 2019 03:34:01 +0100 (CET) Received: (qmail 94605 invoked by uid 500); 6 Mar 2019 02:34:00 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 94580 invoked by uid 99); 6 Mar 2019 02:34:00 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Mar 2019 02:34:00 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id B17E6877EA; Wed, 6 Mar 2019 02:33:59 +0000 (UTC) Date: Wed, 06 Mar 2019 02:33:59 +0000 To: "commits@zookeeper.apache.org" Subject: [zookeeper] branch branch-3.5 updated: ZOOKEEPER-3253: client should not send requests with cxid=-4, -2, or -1 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155183963960.22428.14322616895602700823@gitbox.apache.org> From: phunt@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: zookeeper X-Git-Refname: refs/heads/branch-3.5 X-Git-Reftype: branch X-Git-Oldrev: 89e2c12ad7b72e57e15761cd348af8de5d09b602 X-Git-Newrev: 5d44251c251ecc90288d71bd1e199d097aa4a37a X-Git-Rev: 5d44251c251ecc90288d71bd1e199d097aa4a37a X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. phunt pushed a commit to branch branch-3.5 in repository https://gitbox.apache.org/repos/asf/zookeeper.git The following commit(s) were added to refs/heads/branch-3.5 by this push: new 5d44251 ZOOKEEPER-3253: client should not send requests with cxid=-4, -2, or -1 5d44251 is described below commit 5d44251c251ecc90288d71bd1e199d097aa4a37a Author: Samuel Just AuthorDate: Tue Mar 5 18:32:04 2019 -0800 ZOOKEEPER-3253: client should not send requests with cxid=-4, -2, or -1 - Add test for cxid rollover to 1 - Modify ClientCnxn.SendThread.getXid() to increment from MAX to 1. Author: Samuel Just Reviewers: phunt@apache.org Closes #787 from athanatos/forupstream/ZOOKEEPER-3253 Change-Id: Ib3d111170bb086d6982f2cf0ee5cf8afd5157588 (cherry picked from commit e10c93a590cc1b73eebad48d18cfcbceb3ec0d4d) Signed-off-by: Patrick Hunt --- .../main/java/org/apache/zookeeper/ClientCnxn.java | 9 +++++- .../org/apache/zookeeper/TestableZooKeeper.java | 34 +++++++++++++++++++++ .../java/org/apache/zookeeper/test/ClientTest.java | 35 ++++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java index 9889fac..b15ec53 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java @@ -1486,7 +1486,8 @@ public class ClientCnxn { } } - private int xid = 1; + // @VisibleForTesting + protected int xid = 1; // @VisibleForTesting volatile States state = States.NOT_CONNECTED; @@ -1496,6 +1497,12 @@ public class ClientCnxn { * the server. Thus, getXid() must be public. */ synchronized public int getXid() { + // Avoid negative cxid values. In particular, cxid values of -4, -2, and -1 are special and + // must not be used for requests -- see SendThread.readResponse. + // Skip from MAX to 1. + if (xid == Integer.MAX_VALUE) { + xid = 1; + } return xid++; } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java b/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java index bb1bd12..34c8c0c 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java @@ -26,6 +26,7 @@ import java.util.concurrent.TimeUnit; import org.apache.jute.Record; import org.apache.zookeeper.admin.ZooKeeperAdmin; +import org.apache.zookeeper.client.HostProvider; import org.apache.zookeeper.proto.ReplyHeader; import org.apache.zookeeper.proto.RequestHeader; @@ -35,6 +36,39 @@ public class TestableZooKeeper extends ZooKeeperAdmin { Watcher watcher) throws IOException { super(host, sessionTimeout, watcher); } + + class TestableClientCnxn extends ClientCnxn { + TestableClientCnxn(String chrootPath, HostProvider hostProvider, int sessionTimeout, ZooKeeper zooKeeper, + ClientWatchManager watcher, ClientCnxnSocket clientCnxnSocket, boolean canBeReadOnly) + throws IOException { + super(chrootPath, hostProvider, sessionTimeout, zooKeeper, watcher, + clientCnxnSocket, 0, new byte[16], canBeReadOnly); + } + + void setXid(int newXid) { + xid = newXid; + } + + int checkXid() { + return xid; + } + } + + protected ClientCnxn createConnection(String chrootPath, + HostProvider hostProvider, int sessionTimeout, ZooKeeper zooKeeper, + ClientWatchManager watcher, ClientCnxnSocket clientCnxnSocket, + boolean canBeReadOnly) throws IOException { + return new TestableClientCnxn(chrootPath, hostProvider, sessionTimeout, this, + watcher, clientCnxnSocket, canBeReadOnly); + } + + public void setXid(int xid) { + ((TestableClientCnxn)cnxn).setXid(xid); + } + + public int checkXid() { + return ((TestableClientCnxn)cnxn).checkXid(); + } @Override public List getChildWatches() { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientTest.java index c337e3c..0b5b3c8 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientTest.java @@ -26,6 +26,8 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.zookeeper.AsyncCallback; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.KeeperException.Code; @@ -868,4 +870,37 @@ public class ClientTest extends ClientBase { Assert.assertFalse(zooKeeper.getState().isAlive()); } + + @Test + public void testCXidRollover() throws Exception { + TestableZooKeeper zk = null; + try { + zk = createClient(); + zk.setXid(Integer.MAX_VALUE - 10); + + zk.create("/testnode", "".getBytes(), Ids.OPEN_ACL_UNSAFE, + CreateMode.PERSISTENT); + for (int i = 0; i < 20; ++i) { + final CountDownLatch latch = new CountDownLatch(1); + final AtomicInteger rc = new AtomicInteger(0); + zk.setData("/testnode", "".getBytes(), -1, + new AsyncCallback.StatCallback() { + @Override + public void processResult(int retcode, String path, Object ctx, Stat stat) { + rc.set(retcode); + latch.countDown(); + } + }, null); + Assert.assertTrue("setData should complete within 5s", + latch.await(zk.getSessionTimeout(), TimeUnit.MILLISECONDS)); + Assert.assertEquals("setData should have succeeded", Code.OK.intValue(), rc.get()); + } + zk.delete("/testnode", -1); + Assert.assertTrue("xid should be positive", zk.checkXid() > 0); + } finally { + if (zk != null) { + zk.close(); + } + } + } }