it seems that onControllerFailover is already protected by the controllerLock. The elect() API of ZookeeperLeaderElector is invoked in 3 places and each of those acquires the controllerLock
You are right. The real issue is not that onControllerFailover is not synchronized, but is that the sendRequest is asynchronized. Hence in onControllerFailover, it just put the request on the queue, and while the send thread wakes up to send the message, it may have already been closed by the handleNewSession procedure.
I think the correct fix should be, in ControllerChannelManager.removeExistingBroker, we should also clear the request queue.