You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by GitBox <gi...@apache.org> on 2022/04/21 06:36:33 UTC

[GitHub] [mina-sshd] tomaswolf commented on a diff in pull request #218: [SSHD-1261] Fixed possible pending unfinished pending write request if ChannelAsyncOutputStream closed prematurely

tomaswolf commented on code in PR #218:
URL: https://github.com/apache/mina-sshd/pull/218#discussion_r854828577


##########
sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelAsyncOutputStream.java:
##########
@@ -127,6 +129,28 @@ protected CloseFuture doCloseGracefully() {
         return builder().when(pendingWrite.get()).build().close(false);
     }
 
+    @Override
+    protected void doCloseImmediately() {
+        Channel channel = getChannel();
+        long channelId = (channel == null) ? -1L : channel.getChannelId();
+        abortCurrentWrite(() -> new SshChannelClosedException(channelId, "Channel closed before pending write completed"));
+
+        super.doCloseImmediately();
+    }
+
+    protected synchronized IoWriteFutureImpl abortCurrentWrite(Supplier<? extends Exception> errorProvider) {
+        IoWriteFutureImpl future = pendingWrite.get();
+        if ((future != null) && (!future.isDone())) {
+            Exception error = errorProvider.get();
+            log.debug("abortCurrentWrite({}) aborting pending write={} - {} : {}",
+                    this, future, error == null ? null : error.getClass().getSimpleName(),
+                    error == null ? null : error.getMessage());
+            future.setValue(error);
+        }
+
+        return future;
+    }

Review Comment:
   Terminating the current future is probably right, but still I don't think this will help in all situations. On a graceful shutdown, `AbstractCloseable.close` will call `doCloseGracefully()` *first*, which will block until the pending future is fulfilled.
   
   However, it also calls `preClose()` first, which will already close the `packetWriter`.
   
   I'm not sure a `ChannelAsyncOutputStream` *can* be closed gracefully in this situation.



-- 
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: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org