Oipo commented on a change in pull request #245:
URL: https://github.com/apache/celix/pull/245#discussion_r436576299
##########
File path: bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_handler.c
##########
@@ -501,10 +496,13 @@ static inline int pubsub_tcpHandler_closeConnectionEntry(
}
}
if (entry->fd >= 0) {
+
+ celixThreadRwlock_unlock(&handle->dbLock);
Review comment:
So the possible issue this might introduce is that entry might be free'd
in the meantime, when this particular function releases control of the mutex.
But the issue with having the lock while doing the
`receiverDisconnectMessageCallback`, is that it might give deadlocks due to
different lock orders of mutexes. That's why the previous method was to
sometimes instruct `receiverDisconnectMessageCallback` to not lock. But this
gives reproducable data races due to modifying a hashmap in multiple threads.
Really the solution here would be to use one mutex for both the admin and
the receiver/subscribers, at the expense of latency/throughput. But I honestly
don't know of any other foolproof way to fix these issues.
----------------------------------------------------------------
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:
[email protected]
