Paragrf commented on code in PR #3378:
URL: https://github.com/apache/kvrocks/pull/3378#discussion_r2882020158


##########
src/server/worker.cc:
##########
@@ -405,8 +417,8 @@ void Worker::MigrateConnection(Worker *target, 
redis::Connection *conn) {
   }
   bufferevent_base_set(target->base_, bev);
   conn->SetCB(bev);
-  bufferevent_enable(bev, EV_READ | EV_WRITE);
   conn->SetOwner(target);
+  bufferevent_enable(bev, EV_READ | EV_WRITE);

Review Comment:
   The old execution order caused a race condition:
   
   - Sequence: bufferevent_enable → SetOwner.
   
   - Issue: The event may trigger instantly after enable.The callback finds the 
old worker in owner_, so conn->Owner() provides incorrect data.paused_conns_ 
stores the wrong worker, leading to invalid routing and potential crashes 
during the unpause phase.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to