To answer the question raised in this comment I started to look at whether we could block on the closeLock and I think the answer is no but also I now have more questions about concurrency during closing. Can we definitely not add to acceptedChannels after the server channel is closed? Also this comment:
|
closeLock.readLock().lock(); // ensure we don't open connections while we are closing |
Does it really do that? It did in the past when we blocked the thread while the connection completes, but today I think it's possible for us to open a connection and then leak it by shutting the event loop down. It'll be ok for connections that are happening via ClusterConnectionManager#connectToNode since we closed all the connection managers down first, but we bypass that mechanism for discovery and sniffing.
Not sure there's really a bug here, I haven't tried to reproduce it or anything. If there is a bug then it's benign in production anyway since we're shutting down so everything will be cleaned up soon, but it's still untidy and might cause test issues.
To answer the question raised in this comment I started to look at whether we could block on the
closeLockand I think the answer is no but also I now have more questions about concurrency during closing. Can we definitely not add toacceptedChannelsafter the server channel is closed? Also this comment:elasticsearch/server/src/main/java/org/elasticsearch/transport/TcpTransport.java
Line 282 in e6fd459
Does it really do that? It did in the past when we blocked the thread while the connection completes, but today I think it's possible for us to open a connection and then leak it by shutting the event loop down. It'll be ok for connections that are happening via
ClusterConnectionManager#connectToNodesince we closed all the connection managers down first, but we bypass that mechanism for discovery and sniffing.Not sure there's really a bug here, I haven't tried to reproduce it or anything. If there is a bug then it's benign in production anyway since we're shutting down so everything will be cleaned up soon, but it's still untidy and might cause test issues.