From 2610a19695b4598bded1611e086b29a96151dad2 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Wed, 6 Apr 2022 13:21:39 +0530 Subject: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value When packets are added to ClientCnxn's outgoing packet queue we ensure there's no conflict with an ongoing flush of that queue because of connection loss. Synchronization used to be on the state field's value. This value is both not stable (its value changes over time), possibly causing improper synchronization, and global, which can cause contention in applications that run several ZooKeeper clients. We now synchronize on outgoingQueue which is both local to a ClientCnxn's instance and stable. Author: Sylvain Wallez Reviewers: maoling , Mohammad Arshad Closes #1257 from swallez/ZOOKEEPER-3652 and squashes the following commits: 82e2cad2c [Sylvain Wallez] Instruct SpotBugs that we know what we're doing when synchronizing on outgoingQueue b0bc03d6f [Sylvain Wallez] ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value (cherry picked from commit 91e0520133b82acb87ab60962fce5eae992d87e8) Author: Sylvain Wallez Author: Mate Szalay-Beko Reviewers: Sylvain Wallez , Mohammad Arshad , maoling Closes #1850 from symat/ZOOKEEPER-3652-branch-3.5 and squashes the following commits: 5cd29db5f [Mate Szalay-Beko] remove formatting changes e925265e0 [Sylvain Wallez] ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value --- zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 index 4f9d18b80..5341f5761 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java @@ -1114,6 +1114,7 @@ public class ClientCnxn { private static final String RETRY_CONN_MSG = ", closing socket connection and attempting reconnect"; @Override + @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER") public void run() { clientCnxnSocket.introduce(this, sessionId, outgoingQueue); clientCnxnSocket.updateNow(); @@ -1256,7 +1257,7 @@ public class ClientCnxn { } } } - synchronized (state) { + synchronized (outgoingQueue) { // When it comes to this point, it guarantees that later queued // packet to outgoingQueue will be notified of death. cleanup(); @@ -1589,6 +1590,7 @@ public class ClientCnxn { ctx, watchRegistration, null); } + @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER") public Packet queuePacket(RequestHeader h, ReplyHeader r, Record request, Record response, AsyncCallback cb, String clientPath, String serverPath, Object ctx, WatchRegistration watchRegistration, @@ -1608,7 +1610,7 @@ public class ClientCnxn { // 1. synchronize with the final cleanup() in SendThread.run() to avoid race // 2. synchronized against each packet. So if a closeSession packet is added, // later packet will be notified. - synchronized (state) { + synchronized (outgoingQueue) { if (!state.isAlive() || closing) { conLossPacket(packet); } else { -- cgit v1.2.1