Reset the sequence numbers on Session disconnect to support reconnection

[mvegter]

Resolves #408

[mvegter]

Can also be reproduced by changing any of the Algorithms IT to do :

    try {
      session.connect(timeout);
      assertTrue(session.isConnected());

      // Verify reconnect
      session.disconnect();
      session.connect(timeout);
      assertTrue(session.isConnected());

    } finally {
      session.disconnect();
    }

[norrisjeremy]

I wonder if we should wrap this in a assertDoesNotThrow() to emphasize that we expect this to succeed?

[mvegter]

The test already fails if an Exception is thrown, so right now it is indeed implied. Let me know if you want that changed or not , i don't really see that as a common pattern in the current ITs

[norrisjeremy]

I wouldn't be surprised if the intention was for the Session object to not be reusable to create a new connection after the disconnect method has been called. Why not instead simply create a new Session object instead of trying to reuse the existing one?

Also considering this isn't a regression per se (since JSch has behaved in this manner for years), I would lean more towards not merging this in, since there is always a chance that this could instead introduce a some other new regression of some sort instead.

@mwiede what do you think?

[mvegter]

I wouldn't be surprised if the intention was for the Session object to not be reusable to create a new connection after the disconnect method has been called. Why not instead simply create a new Session object instead of trying to reuse the existing one?

I understand your position on this change, however the reason for introducing this is that the Session already resets the isConnected flag. This technically allows anyone to call connect() again , whereas a connected session throws an Exception if we attempt to re-connect. If we were to disallow this I would expect (perhaps in a future change) to throw an Exception when calling connect() after a disconnect()

On another note, allowing a client to call connect() multiple times can also allow for more resilience with retries on certain IOException without setting up a new Jsch/Session .

[norrisjeremy]

I don't feel strongly enough about this to block it, as long as @mwiede is also ok with it.

[mwiede]

we do not really know, whether the Session was intened for re-use or not, but I think we can accept this change, because it also cannot be seen in the code, that reuse has to be blocked.