mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Re: Please help review and test solution for SSHD-849
Date Fri, 19 Oct 2018 08:39:40 GMT
Hi Lyor,

a few hints, onsidering I haven't reviewed the code...

Le 19/10/2018 à 07:00, Lyor Goldstein a écrit :
> Here is the issue in a nutshell - a client might open an SSH tunnel, send
> some data and close (normally) its side of the tunnel before the channel to
> the other side has been successfully established and all data transmitted.
> Currently a race condition may occur in such a scenario where the code
> closes the channel while pending messages are still in transit.
> The patch attempts to accumulate pending messages until channel is open
> (successfully) and then flush them while maintaining their *order*. At the
> same time, it attempts to "delay" other messages that may arrive while
> flushing is in progress so that they will also be sent in the correct
> order. It also delays the closing of the tunnel until pending data has been
> flushed (unless an error occurs during the flush...).

You should not have to deal with the delayed closing: MINA is already
allowing you to do that, if you call closeOnFlush() instead of
closeNow() -or close(), which maps to closeNow()-. It will then flush
all the pending messages before closing the session. No message written
in the session *after* you have called close(false) will be sent.

At the same time, as soon as the closeOnFlush() method is called, we
*should* stop reading anything from the client - ie set remove OP_READ
flag from the selectionKey, something MINA is not doing (afaict). This
is most certainly a missing part of the closeOnFlush(). The problem is
that the OP_READ flag is registrered everytime we process the session,
until the session is in CLOSING mode. That means we should probably set
the session in CLOSING mode when we call closeOnFlush(), but we can't
because this is a computed flag :/ So it boils down to check the session
state - and we probably will need to add a additional flag like
CLOSE_ON_FLUSH- when we try to update the event a channel is interested in.

I would suggest filling a JIRA ticket for this one, it requires some
further investigation, but I suspect it would ease yur implementation A LOT.

Emmanuel Lecharny


View raw message