tomaswolf commented on PR #244: URL: https://github.com/apache/mina-sshd/pull/244#issuecomment-1246549726
This looks OK now. - The problem with `ClientDeadlockTest` was [this](https://github.com/apache/mina-sshd/pull/244/commits/331018f822ccc072acdbcc5d103bfaff0a77ce37#diff-081006a0bb09ef97f2c22a6930fdda78ddfdbbc50cd44566cd254163488c66b8L263), as explained in the commit message. - `ClientTest.testClientDisconnect()` then also got a spurious failure; the problem there was essentially [this](https://github.com/apache/mina-sshd/pull/244/commits/ff247f539980bfe2231bddf7eaeb809f5c675002#diff-859aa84d4bf7ccdee8fabf7ec1c3b6f77574de417fd7cf308f3b54f0a4b5fe03R1422): the client closed the session, and the `suspend()` call came when the session was already closed and the `NettyIoSession.context` was already `null`. Fixed by ensuring thread safety for accesses to the `ChannelHandlerContext`, and by making the test invoke `suspend()` earlier. Some of the test runs done for the PR also had failures of `PortForwardingTest.testRemoteForwardingSecondTimeInSameSession()` when run with MINA. The cause is unclear, and it seems to be a spuriously flaky test. The test tries to re-use a previously bound port. Although SO_REUSEADDR is set, there may be a timing issue. If there is, it would be in Java/NIO or even at the OS socket level. Maybe if one tries to re-use a socket too quickly, it can still fail. I'd suggest to merge this PR as is, and if that port forwarding test starts failing frequently, re-open SSHD-1256 and try to fix in a separate change. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
