ZOOKEEPER-4537: Race between SyncThread and CommitProcessor thread

Zookeeper server can get stuck when it has just one client and the only way it recovers is due to a socket timeout or another client commit request.
Sync thread reads commitIsWaiting outside of a sync block and acts on this information in a sync block later. The actual status of commitIsWaiting can change between the time where commitIsWaiting is read and acted upon because commit thread updates it outside a sync block.
Fix here is to ensure that we read and process commitIsWaiting inside a sync block.

https://issues.apache.org/jira/browse/ZOOKEEPER-4537

Author: jithin23 <jithin.girish@gmail.com>

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

Closes #1877 from jithin23/ZOOKEEPER-4537

1 files changed, 5 insertions, 6 deletions

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
index 86dce2b7f..11ba09361 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
@@ -213,12 +213,11 @@ public class CommitProcessor extends ZooKeeperCriticalThread implements RequestP
                  * request from a client on another server (i.e., the order of
                  * the following two lines is important!).
                  */
-                commitIsWaiting = !committedRequests.isEmpty();
-                requestsToProcess = queuedRequests.size();
-                // Avoid sync if we have something to do
-                if (requestsToProcess == 0 && !commitIsWaiting) {
-                    // Waiting for requests to process
-                    synchronized (this) {
+                synchronized (this) {
+                    commitIsWaiting = !committedRequests.isEmpty();
+                    requestsToProcess = queuedRequests.size();
+                    if (requestsToProcess == 0 && !commitIsWaiting) {
+                        // Waiting for requests to process
                         while (!stopped && requestsToProcess == 0 && !commitIsWaiting) {
                             wait();
                             commitIsWaiting = !committedRequests.isEmpty();