mina
mina copied to clipboard
apache
[DIRMINA-1169] Fix unbinding a serverSocketChannel
Since Java 11, closing a ServerSocketChannel may release the socket only on the next select() call. If we try to register a new binding for an unbound socket before the next select, a BindException may be thrown.
Ensure that there is a select() call between unbinding some sockets and binding new sockets. Mark the unbind futures as "done" only after that intervening select().
Test plan:
- check out this PR
- replace NioSocketAcceptor by the 2.0.23 version
- run the SocketAcceptorTest on a JVM 11 (or newer). It should fail with a BindException.
- git reset --hard
- run the SocketAcceptorTest on a JVM 11 (or newer) again. It should pass.
Note that if SocketAcceptorTest is run on a JVM 8, it will succeed in either case.
Would be good if this could also be merged to 2.2.x then. It causes intermittent failures in our test suite (QuickFIX/J)
@elecharny could you please take look and merge this if you think it's OK? And then merge or cherry-pick it to the 2.1.X and 2.2.X branches? I'm a bit confused by the git history of this repo, and it's not clear to me how you manage back-ports or fixes in old versions, otherwise I might be able to do so myself. The three 2.* branches seem to be completely separate. That's not what I'm used to from EGit or JGit, where after a fix on an old maintenance branch we merge the change up to the current master/main branch.
Having a 2.0.24 including the fix would also be useful. Currently we have disabled one test in Apache MINA sshd because of this problem.
Hi Thomas,
one slight problem with the AbstractPollingIoAcceptor.handleUnbound() method in 2.0.X: it won't compile because teh code is expected to be 1.7 compatible. The method is:
protected void handleUnbound(Collection<AcceptorOperationFuture> unboundFutures) throws Exception {
unboundFutures.forEach(AcceptorOperationFuture::setDone);
}
The :: operator is unknown in Java 7. We need to go back to a standard lambda. That will not be an issue in 2.2.X, but still is in 2.1.X.
Ok I unfolded the '::' and replaced it with a classic 'for' loop.
Patch applied in 2.0.X branch.
I'll see if it can be ported to 2.1 and 2.2 branch.
Thanks !
Why Java 7?? The pom sets maven.source.version and maven.target.versionto 8, and when I imported mina-core into Eclipse, the projects got of course set up with Java 8.
And that's on the 2.0.X branch.
Because of line 802 and following:
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>${version.compiler.plugin}</version>
<configuration>
<encoding>UTF-8</encoding>
<source>1.7</source>
<target>1.7</target>
<debug>true</debug>
<optimize>true</optimize>
<showDeprecations>true</showDeprecations>
</configuration>
</plugin>
It's fixed, btw.
Missed that. And anyway maven.source.version"8" is likely wrong, shouldn't that be "1.8" if at all? Now I wonder why I got a Java 8 setup in Eclipse...
BTW, I just noticed commit 2331425ae979c in 2.1.X. Don't know what the problems there were, but that test repeatedly unbinds and binds the same port, too. Perhaps it also ran into this race condition.
@elecharny Any chance that this could be integrated into 2.1.x (and later 2.2.x) and released? It causes spurious failures in our test suite. Thank you in advance