RobertIndie commented on code in PR #551:
URL: https://github.com/apache/pulsar-client-cpp/pull/551#discussion_r2911492607
##########
lib/ConnectionPool.cc:
##########
@@ -61,8 +63,19 @@ bool ConnectionPool::close() {
if (cnx) {
// The 2nd argument is false because removing a value during the
iteration will cause segfault
cnx->close(ResultDisconnected, false);
Review Comment:
`ConnectionPool::close` holds the pool lock while iterating over the
connections.
However, when a connection is closed with `detach`, the following path will
be triggered:
https://github.com/apache/pulsar-client-cpp/blob/5b4a9143aeb4623b75bb9a5875188f50c07d71e0/lib/ClientConnection.cc#L1389
This eventually calls `pool_.remove`, which tries to acquire the same lock
again:
https://github.com/apache/pulsar-client-cpp/blob/c543840ce12a37c15157f4c4d6706e0885ffb96b/lib/ConnectionPool.cc#L152
Since `ConnectionPool::close` is still holding the lock while waiting for
`pendingOperations`, this could lead to a temporary deadlock.
##########
lib/ClientConnection.cc:
##########
Review Comment:
For this path, `pendingOperations_` will not be increased when `sendCommand`
returns. It is only increased in `sendCommandInternal` after this callback is
executed.
This means the issue that existed before this PR can still happen: when the
connection is being closed, there might still be an in-flight callback that
hasn't been counted yet.
--
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]
