[GitHub] [zookeeper] muse-dev[bot] commented on a change in pull request #1653: ZOOKEEPER-4260: Backport ZOOKEEPER-3575 to branch-3.6

[GitBox <gi...@apache.org>]

muse-dev[bot] commented on a change in pull request #1653:
URL: https://github.com/apache/zookeeper/pull/1653#discussion_r600932669



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
##########
@@ -152,13 +165,27 @@ void validateSession(ServerCnxn cnxn, long clientId, int timeout) throws IOExcep
     }
 
     /**
-     * write a packet to the leader
+     * write a packet to the leader.
+     *
+     * This method is called by multiple threads. We need to make sure that only one thread is writing to leaderOs at a time.
+     * When packets are sent synchronously, writing is done within a synchronization block.
+     * When packets are sent asynchronously, sender.queuePacket() is called, which writes to a BlockingQueue, which is thread-safe.
+     * Reading from this BlockingQueue and writing to leaderOs is the learner sender thread only.
+     * So we have only one thread writing to leaderOs at a time in either case.
      *
      * @param pp
      *                the proposal packet to be sent to the leader
      * @throws IOException
      */
     void writePacket(QuorumPacket pp, boolean flush) throws IOException {
+        if (asyncSending) {
+            sender.queuePacket(pp);
+        } else {
+            writePacketNow(pp, flush);
+        }
+    }
+
+    void writePacketNow(QuorumPacket pp, boolean flush) throws IOException {
         synchronized (leaderOs) {

Review comment:
       *THREAD_SAFETY_VIOLATION:*  Read/Write race. Non-private method `Learner.writePacketNow(...)` reads without synchronization from `this.leaderOs`. Potentially races with write in method `Learner.connectToLeader(...)`.
    Reporting because this access may occur on a background thread.
   (at-me [in a reply](https://docs.muse.dev/docs/talk-to-muse/) with `help` or `ignore`)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org