From 6ffe45e4c78b01c72a83d0c7ec0807e8e74410e5 Mon Sep 17 00:00:00 2001 From: pasynkov Date: Thu, 2 May 2024 16:56:40 +0200 Subject: [PATCH 1/3] Nio2Session: Do not silence write exceptions Previously, if any exception occurs during write, the corresponding future was set as successful. This commit fixes behaviour and sets future as completed with exception --- .../apache/sshd/common/io/nio2/Nio2Session.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java index 18819e5a3..23493e801 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java @@ -516,7 +516,8 @@ protected void startWriting() { doWriteCycle(buffer, handler); } } catch (Throwable e) { - future.setWritten(); + future.setException(e); + finishWrite(future); if (e instanceof RuntimeException) { throw (RuntimeException) e; @@ -555,17 +556,22 @@ protected void handleCompletedWriteCycle( Nio2CompletionHandler completionHandler, Integer result, Object attachment) { if (buffer.hasRemaining()) { try { + if (log.isDebugEnabled()) { + log.debug("handleCompletedWriteCycle({}) incomplete write of writeLen={}. Written result={}, resume writing buffer.remaining()={} at cycle={} after {} nanos", + this, writeLen, result, buffer.remaining(), writeCyclesCounter.get(), System.nanoTime() - lastWriteCycleStart.get()); + } + socket.write(buffer, null, completionHandler); } catch (Throwable t) { - debug("handleCompletedWriteCycle({}) {} while writing to socket len={}: {}", - this, t.getClass().getSimpleName(), writeLen, t.getMessage(), t); - future.setWritten(); + warn("handleCompletedWriteCycle({}) {} while writing to socket len={}, result={}: {}", + this, t.getClass().getSimpleName(), writeLen, result, t.getMessage(), t); + future.setException(t); finishWrite(future); } } else { if (log.isTraceEnabled()) { log.trace("handleCompletedWriteCycle({}) finished writing len={} at cycle={} after {} nanos", - this, writeLen, writeCyclesCounter, System.nanoTime() - lastWriteCycleStart.get()); + this, writeLen, writeCyclesCounter.get(), System.nanoTime() - lastWriteCycleStart.get()); } // This should be called before future.setWritten() to avoid WriteAbortedException From 43dbbf4f00b59b92a350d785a2b7d75626b56f20 Mon Sep 17 00:00:00 2001 From: pasynkov Date: Thu, 2 May 2024 17:07:02 +0200 Subject: [PATCH 2/3] Nio2Session: Use better structure for writes queue ConcurrentLinkedQueue is more suitable here - it ensures add/poll synchronization. The continuation handler is called from multiple threads, and it is good to have synchronized access - no specific from previously TransferQueue is used --- .../main/java/org/apache/sshd/common/io/nio2/Nio2Session.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java index 23493e801..c21cd24a7 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java @@ -29,7 +29,7 @@ import java.util.Map; import java.util.Objects; import java.util.Queue; -import java.util.concurrent.LinkedTransferQueue; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -65,7 +65,7 @@ public class Nio2Session extends AbstractCloseable implements IoSession { private final SocketAddress remoteAddress; private final SocketAddress acceptanceAddress; private final PropertyResolver propertyResolver; - private final Queue writes = new LinkedTransferQueue<>(); + private final Queue writes = new ConcurrentLinkedQueue<>(); private final AtomicReference currentWrite = new AtomicReference<>(); private final AtomicLong readCyclesCounter = new AtomicLong(); private final AtomicLong lastReadCycleStart = new AtomicLong(); From 1f1a80204c05f96c0715a1937fa36e8bb79fb901 Mon Sep 17 00:00:00 2001 From: pasynkov Date: Thu, 2 May 2024 17:22:20 +0200 Subject: [PATCH 3/3] Nio2Session: Multi-part write honors timeout If the requested buffer was not written in a single chunk and the handleCompletedWriteCycle processes incomplete write, all next writes were called without configured timeout. This commit fixes the behaviour and set timeout for next "socket.write" calls. Note: The configured timeout is used for each data chunk, meaning that depending on underlying network layer the actual buffer write may take longer than timeout --- .../main/java/org/apache/sshd/common/io/nio2/Nio2Session.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java index c21cd24a7..f6f14ae95 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java @@ -561,7 +561,7 @@ protected void handleCompletedWriteCycle( this, writeLen, result, buffer.remaining(), writeCyclesCounter.get(), System.nanoTime() - lastWriteCycleStart.get()); } - socket.write(buffer, null, completionHandler); + doWriteCycle(buffer, completionHandler); } catch (Throwable t) { warn("handleCompletedWriteCycle({}) {} while writing to socket len={}, result={}: {}", this, t.getClass().getSimpleName(), writeLen, result, t.getMessage(), t);