[
https://issues.apache.org/jira/browse/SSHD-1262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17534773#comment-17534773
]
Thomas Wolf commented on SSHD-1262:
-----------------------------------
I'm not happy with the current implementation of TCP/IP forwarding. I see
several problems:
* {{Nio2Session.resumeRead()}} is broken. If called in a future listener
(which is the most natural place to call this!), it may lead to concurrent
reads on the port and a {{ReadPendingException}} being thrown. Also note that
the Mina and Netty back-ends don't implement suspendRead/resumeRead. Moreover,
with NIO2, the framework may perform synchronous reads if data is available (up
to 16 completion handlers on the call stack; configurable with system property
{{sun.nio.ch.maxCompletionHandlersOnStack}}; there's also another property
{{sun.nio.ch.disableSynchronousRead}} (a {{boolean}}, by default {{false}})
that can be set to {{true}} to always force asynchronous reads). If
{{resumeRead()}} is called in a future listener, that listener may be invoked
on a different NIO thread, and if {{resumeRead()}} then causes synchronous
reads, the thread will not return from the original handler until the NIO
framework happens to decide to do a truly asynchronous read. So the original
session that fulfilled the future may be blocked for up to 16 reads on the
forwarded port.
* I see buffering galore when reading from ports on both sides ("client" and
"server" parts of the channel implementation; here "client" and "server" does
not refer to the SSH client or server but the side of the forwarding channel
that is implemented). IMO this is just wrong; there should be no buffering at
all. Instead perform the next read on the port only when the currently read
data has been written to the channel. Push back onto the producer if it is too
fast; don't try to accommodate a fast producer by piling up buffered data
inside Apache MINA sshd. This buffering has led to problems in the past
(SSHD-1070, SSHD-1125). It makes the code much more complicated, has the risk
of unbounded memory consumption inside Apache MINA sshd, and we have to worry
about making sure to flush out all buffered data before a port forwarding is
torn down. If we don't buffer, we don't have these problems to that extent.
(We'd just have to make sure that the last incoming data we read from the port
gets written before shutting down the SSH channel.)
* Using a synchronous {{ChannelOutputStream}} to write data read from the port
into the channel is IMO just plain wrong. It blocks when the channel window is
exhausted. This means the NIO thread handling the read from the port and doing
the forwarding write is blocked until some other NIO thread receives a window
adjust for that channel. If there are more port forwardings than there are NIO
threads (fixed thread pool), I think this may lead to a situation where all NIO
threads are blocked waiting for window adjustments, and none left over to
actually handle the incoming window adjustments. Writing to a channel in port
forwarding should IMO _always_ and unconditionally use
{{ChannelAsyncOutputStream}}. That one doesn't block the writing thread, but
just puts the write "on hold".
* I don't see buffering when transferring data from the SSH channel to the
port. (Which I think is good.) But what if data is coming in through the SSH
channel faster than it can be written to the port? I didn't look at it yet, but
this direction needs to manage channel flow control to match the speed of
writing to the port. The channel window must be such that the side that send
data into the channel doesn't send faster than the receiving side can write to
the port.
I do think this forwarding needs a rewrite to avoid buffering; use
{{ChannelAsyncOutputStream}} and suspend reading from the port until data has
been written.
I have a first prototype for the {{TcpipClientChannel}} doing exactly this
working with NIO2. Have to test now with Mina and Netty. Then in a second step
looks at the other side of the channel ({{TcpipServerChannel}}) and do the same
there.
> Unhandled SSH_MSG_CHANNEL_WINDOW_ADJUST leeds to SocketTimeoutException
> -----------------------------------------------------------------------
>
> Key: SSHD-1262
> URL: https://issues.apache.org/jira/browse/SSHD-1262
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 2.7.0, 2.8.0
> Reporter: Stefan Maute
> Priority: Major
> Labels: mina, sshd
> Attachments: failing-pf-trace-logs.txt
>
>
> Wanted to open a local port forwarding to an SSH server and afterwards
> connect to MQTT server over the SSh tunnel. During the MQTT connection
> attempt I encountered a SocketTimeoutException in the logs (StackTrace below).
> What I could see in the logs is that the server is sending an initial window
> size of 0 and afterwards the _waitForCondition_ method is throwing the
> SocketTimeoutException .
> After analyzing the logs I maybe have an idea what is going on here. My
> assumption is that there is some deadlock in case the initial window size is
> 0 and the SSH_MSG_CHANNEL_WINDOW_ADJUST isn't handled properly.
> I have attached the whole log file. Lines of interest are from 169 to 229.
> Tested with 2.7.0 and 2.8.0.
>
> {code:java}
> WARN org.apache.sshd.common.forward.DefaultForwarder -
> exceptionCaught(Nio2Session[local=/127.0.0.1:44259, remote=/127.0.0.1:60188])
> SocketTimeoutException:
> waitForCondition(Window[client/remote](TcpipClientChannel[id=0,
> recipient=0]-ClientSessionImpl[Admin@/x.xxx.xx.x:xxxx])) timeout exceeded:
> PT30S java.net.SocketTimeoutException:
> waitForCondition(Window[client/remote](TcpipClientChannel[id=0,
> recipient=0]-ClientSessionImpl[Admin@/x.xxx.xx.x:xxxx])) timeout exceeded:
> PT30S at
> org.apache.sshd.common.channel.Window.waitForCondition(Window.java:332)
> at org.apache.sshd.common.channel.Window.waitForSpace(Window.java:286) at
> org.apache.sshd.common.channel.ChannelOutputStream.write(ChannelOutputStream.java:154)
> at
> org.apache.sshd.client.channel.ClientChannelPendingMessagesQueue.writeMessage(ClientChannelPendingMessagesQueue.java:174)
> at
> org.apache.sshd.client.channel.ClientChannelPendingMessagesQueue.flushPendingQueue(ClientChannelPendingMessagesQueue.java:221)
> at
> org.apache.sshd.client.channel.ClientChannelPendingMessagesQueue.operationComplete(ClientChannelPendingMessagesQueue.java:206)
> at
> org.apache.sshd.client.channel.ClientChannelPendingMessagesQueue.operationComplete(ClientChannelPendingMessagesQueue.java:51)
> at
> org.apache.sshd.common.future.AbstractSshFuture.notifyListener(AbstractSshFuture.java:159)
> at
> org.apache.sshd.common.future.DefaultSshFuture.notifyListeners(DefaultSshFuture.java:215)
> at
> org.apache.sshd.common.future.DefaultSshFuture.setValue(DefaultSshFuture.java:112)
> at
> org.apache.sshd.client.future.DefaultOpenFuture.setOpened(DefaultOpenFuture.java:68)
> at
> org.apache.sshd.client.channel.AbstractClientChannel.handleOpenSuccess(AbstractClientChannel.java:360)
> at
> org.apache.sshd.common.session.helpers.AbstractConnectionService.channelOpenConfirmation(AbstractConnectionService.java:545)
> at
> org.apache.sshd.common.session.helpers.AbstractConnectionService.process(AbstractConnectionService.java:456)
> at
> org.apache.sshd.common.session.helpers.AbstractSession.doHandleMessage(AbstractSession.java:503)
> at
> org.apache.sshd.common.session.helpers.AbstractSession.handleMessage(AbstractSession.java:429)
> at
> org.apache.sshd.common.session.helpers.AbstractSession.decode(AbstractSession.java:1466)
> at
> org.apache.sshd.common.session.helpers.AbstractSession.messageReceived(AbstractSession.java:389)
> at
> org.apache.sshd.common.session.helpers.AbstractSessionIoHandler.messageReceived(AbstractSessionIoHandler.java:64)
> at
> org.apache.sshd.common.io.nio2.Nio2Session.handleReadCycleCompletion(Nio2Session.java:359)
> at
> org.apache.sshd.common.io.nio2.Nio2Session$1.onCompleted(Nio2Session.java:336)
> at
> org.apache.sshd.common.io.nio2.Nio2Session$1.onCompleted(Nio2Session.java:333)
> at
> org.apache.sshd.common.io.nio2.Nio2CompletionHandler.lambda$completed$0(Nio2CompletionHandler.java:38)
> at
> java.base/java.security.AccessController.doPrivileged(AccessController.java:318)
> at
> org.apache.sshd.common.io.nio2.Nio2CompletionHandler.completed(Nio2CompletionHandler.java:37)
> at java.base/sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:129) at
> java.base/sun.nio.ch.Invoker$2.run(Invoker.java:221) at
> java.base/sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:113)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
> at java.base/java.lang.Thread.run(Thread.java:833) {code}
>
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
