Skip to content

Commit 99863c2

Browse files
Imporve socketChannel closing in NioConnection (#10895)
1 parent f496ed6 commit 99863c2

File tree

2 files changed

+48
-24
lines changed

2 files changed

+48
-24
lines changed

utils/src/main/java/com/cloud/utils/nio/NioConnection.java

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@
3434
import java.nio.channels.SocketChannel;
3535
import java.security.GeneralSecurityException;
3636
import java.util.ArrayList;
37-
import java.util.HashSet;
3837
import java.util.Iterator;
3938
import java.util.List;
4039
import java.util.Set;
4140
import java.util.concurrent.Callable;
41+
import java.util.concurrent.ConcurrentHashMap;
4242
import java.util.concurrent.ExecutorService;
4343
import java.util.concurrent.Executors;
4444
import java.util.concurrent.Future;
@@ -80,7 +80,7 @@ public abstract class NioConnection implements Callable<Boolean> {
8080
protected ExecutorService _executor;
8181
protected ExecutorService _sslHandshakeExecutor;
8282
protected CAService caService;
83-
protected Set<SocketChannel> socketChannels = new HashSet<>();
83+
protected Set<SocketChannel> socketChannels = ConcurrentHashMap.newKeySet();
8484
protected Integer sslHandshakeTimeout = null;
8585
private final int factoryMaxNewConnectionsCount;
8686
protected boolean blockNewConnections;
@@ -219,7 +219,7 @@ public Boolean call() throws NioConnectionException {
219219
return true;
220220
}
221221

222-
abstract void init() throws IOException;
222+
protected abstract void init() throws IOException;
223223

224224
abstract void registerLink(InetSocketAddress saddr, Link link);
225225

@@ -489,16 +489,47 @@ protected void write(final SelectionKey key) throws IOException {
489489
}
490490

491491
protected void closeConnection(final SelectionKey key) {
492-
if (key != null) {
493-
final SocketChannel channel = (SocketChannel)key.channel();
494-
key.cancel();
492+
if (key == null) {
493+
return;
494+
}
495+
496+
SocketChannel channel = null;
497+
try {
498+
// 1. Check type and handle potential CancelledKeyException
499+
if (key.isValid() && key.channel() instanceof SocketChannel) {
500+
channel = (SocketChannel) key.channel();
501+
}
502+
} catch (CancelledKeyException e) {
503+
logger.trace("Key already cancelled when trying to get channel in closeConnection.");
504+
}
505+
506+
// 2. Cancel the key (safe to call even if already cancelled)
507+
key.cancel();
508+
509+
if (channel == null) {
510+
logger.trace("Channel was null, invalid, or not a SocketChannel for key: " + key);
511+
return;
512+
}
513+
514+
// 3. Try to close the channel if we obtained it
515+
if (channel != null) {
516+
closeChannel(channel);
517+
} else {
518+
logger.trace("Channel was null, invalid, or not a SocketChannel for key: " + key);
519+
}
520+
}
521+
522+
private void closeChannel(SocketChannel channel) {
523+
if (channel != null && channel.isOpen()) {
495524
try {
496-
if (channel != null) {
497-
logger.debug("Closing socket {}", channel.socket());
498-
channel.close();
499-
}
500-
} catch (final IOException ignore) {
501-
logger.info("[ignored] channel");
525+
logger.debug("Closing socket " + channel.socket());
526+
channel.close();
527+
} catch (IOException ignore) {
528+
logger.warn(String.format("[ignored] Exception closing channel: %s, due to %s", channel, ignore.getMessage()));
529+
} catch (Exception e) {
530+
logger.warn(String.format("Unexpected exception in closing channel %s", channel), e);
531+
} finally {
532+
socketChannels.remove(channel);
502533
}
503534
}
504535
}
@@ -530,14 +561,7 @@ public void close(final SelectionKey key) {
530561
/* Release the resource used by the instance */
531562
public void cleanUp() throws IOException {
532563
for (SocketChannel channel : socketChannels) {
533-
if (channel != null && channel.isOpen()) {
534-
try {
535-
logger.info("Closing connection: {}", channel.getRemoteAddress());
536-
channel.close();
537-
} catch (IOException e) {
538-
logger.warn("Unable to close connection due to {}", e.getMessage());
539-
}
540-
}
564+
closeChannel(channel);
541565
}
542566
if (_selector != null) {
543567
_selector.close();

utils/src/main/java/com/cloud/utils/nio/NioServer.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import java.nio.channels.SelectionKey;
2626
import java.nio.channels.ServerSocketChannel;
2727
import java.nio.channels.spi.SelectorProvider;
28-
import java.util.WeakHashMap;
28+
import java.util.concurrent.ConcurrentHashMap;
2929

3030
import org.apache.cloudstack.framework.ca.CAService;
3131

@@ -34,15 +34,15 @@ public class NioServer extends NioConnection {
3434
protected InetSocketAddress localAddress;
3535
private ServerSocketChannel serverSocket;
3636

37-
protected WeakHashMap<InetSocketAddress, Link> links;
37+
protected ConcurrentHashMap<InetSocketAddress, Link> links;
3838

3939
public NioServer(final String name, final int port, final int workers, final HandlerFactory factory,
4040
final CAService caService, final Integer sslHandShakeTimeout) {
41-
super(name, port, workers,factory);
41+
super(name, port, workers, factory);
4242
setCAService(caService);
4343
setSslHandshakeTimeout(sslHandShakeTimeout);
4444
localAddress = null;
45-
links = new WeakHashMap<>(1024);
45+
links = new ConcurrentHashMap<>(1024);
4646
}
4747

4848
public int getPort() {

0 commit comments

Comments
 (0)