Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SSHD-1307] Make Nio2Session.shutdownOutputStream() asynchronous #257

Merged
merged 1 commit into from
Oct 24, 2022

Conversation

tomaswolf
Copy link
Member

Nio2Session.shutdownOutputStream() can be invoked while there are still writes in progress. The output is shut down in TCP/IP port forwarding to propagate an SSH_MSG_CHANNEL_EOF from the channel to whatever is on the other side of the forwarded port; see SSHD-1055. However, writing through the socket is asynchronous, so a channel may get the following sequence of events:

  1. receive last SSH_MSG_CHANNEL_DATA
  2. Nio2Session.writeBuffer() enqueues the write request
  3. SSH_MSG_CHANNEL_DATA handling completes
  4. receive SSH_MSG_CHANNEL_EOF
  5. call Nio2Session.shutdownOutputStream()

If (5) shuts down the output immediately but the write request from (2) has not been written yet, that write attempt will fail with a ClosedChannelException.

The output stream on the socket should not be shut down immediately but only once already pending writes have been done. This is already the case with the MINA and Netty transport back-ends, which only schedule a shutdown request on their write queue.

Implement the same for the NIO2 transport. A write future with a null buffer signifies a shutdown request; true write requests always have a non-null buffer. Make shutdownOutputStream() only enqueue such a shutdown request on the session's write queue; the output will be shut down once startWriting() pops this shutdown request from the write queue.

Nio2Session.shutdownOutputStream() can be invoked while there are still
writes in progress. The output is shut down in TCP/IP port forwarding to
propagate an SSH_MSG_CHANNEL_EOF from the channel to whatever is on the
other side of the forwarded port; see SSHD-1055. However, writing
through the socket is asynchronous, so a channel may get the following
sequence of events:

1. receive last SSH_MSG_CHANNEL_DATA
2. Nio2Session.writeBuffer() enqueues the write request
3. SSH_MSG_CHANNEL_DATA handling completes
4. receive SSH_MSG_CHANNEL_EOF
5. call Nio2Session.shutdownOutputStream()

If (5) shuts down the output immediately but the write request from
(2) has not been written yet, that write attempt will fail with a
ClosedChannelException.

The output stream on the socket should not be shut down immediately but
only once already pending writes have been done. This is already the
case with the MINA and Netty transport back-ends, which only schedule a
shutdown request on their write queue.

Implement the same for the NIO2 transport. A write future with a null
buffer signifies a shutdown request; true write requests always have
a non-null buffer. Make shutdownOutputStream() only enqueue such a
shutdown request on the session's write queue; the output will be shut
down once startWriting() pops this shutdown request from the write
queue.
@tomaswolf tomaswolf merged commit 1f5c0bd into apache:master Oct 24, 2022
@AlexanderAmador
Copy link

I am interested in acquiring this fix in a release. I saw a comment that your next release may be 2.10.0. Do you have an ETA on that release ?

@tomaswolf
Copy link
Member Author

Answer is on SSHD-1307, where you had asked the same.

@tomaswolf tomaswolf deleted the sshd-1307 branch November 2, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants