[ https://issues.apache.org/jira/browse/DIRMINA-1076?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16360060#comment-16360060 ]
Jonathan Valliere commented on DIRMINA-1076: -------------------------------------------- I've thought about it and have come up with the following patch which resolves my question about the previous patch. I was attempting to find a solution which made the smallest number of changes to resolve the problem. As it turns out, moving the {{isDisposing()}} check above {{removeSessions()}} resolves the double-removal issue due to blind removal scheduling based on the Selectors {{keys()}}. I believe the method in the following patch to be safe due to the nature of the disposing state; since disposal is immediate and we don't care about letting Sessions soft-close there is no need to wakeup the Selector. _Patch to prevent double-removal of a single session which causes the counter to go negative preventing the processor from disposing._ {code:java} diff --git a/mina-core/src/main/java/org/apache/mina/core/polling/AbstractPollingIoProcessor.java b/mina-core/src/main/java/org/apache/mina/core/polling/AbstractPollingIoProcessor.java index 79885fa..2715b98 100644 --- a/mina-core/src/main/java/org/apache/mina/core/polling/AbstractPollingIoProcessor.java +++ b/mina-core/src/main/java/org/apache/mina/core/polling/AbstractPollingIoProcessor.java @@ -659,9 +659,20 @@ long currentTime = System.currentTimeMillis(); flush(currentTime); + // Disconnect all sessions immediately if disposal has been + // requested so that we exit this loop eventually. + if (isDisposing()) { + for (Iterator<S> i = allSessions(); i.hasNext();) { + IoSession session = i.next(); + scheduleRemove((S) session); + } + } + // And manage removed sessions nSessions -= removeSessions(); - + + assert nSessions > -1 : "Internal Session Count is Negative"; + // Last, not least, send Idle events to the idle sessions notifyIdleSessions(currentTime); @@ -685,26 +696,6 @@ } assert processorRef.get() == this; - } - - // Disconnect all sessions immediately if disposal has been - // requested so that we exit this loop eventually. - if (isDisposing()) { - boolean hasKeys = false; - - for (Iterator<S> i = allSessions(); i.hasNext();) { - IoSession session = i.next(); - - scheduleRemove((S) session); - - if (session.isActive()) { - hasKeys = true; - } - } - - if (hasKeys) { - wakeup(); - } } } catch (ClosedSelectorException cse) { // If the selector has been closed, we can exit the loop {code} I had to change Christoph's {{AbstractIoServiceTest}} patch because it calls {{connector.dispose(true)}} which the documentation and comments above say is something that should not be done within an {{IoFutureListener}}. The reason I had to change from {{true}} to {{false}} was because {{dispose(true)}} blocks on itself and preventing the {{IoProcessor}} thread from ever stopping. In the described test, this causes the application to create threads until the OS complains about it preventing more threads from being created. _Patch to make the test run in a loop_ {code:java} diff --git a/mina-core/src/test/java/org/apache/mina/core/service/AbstractIoServiceTest.java b/mina-core/src/test/java/org/apache/mina/core/service/AbstractIoServiceTest.java index 2d70f8e..325c4e4 100644 --- a/mina-core/src/test/java/org/apache/mina/core/service/AbstractIoServiceTest.java +++ b/mina-core/src/test/java/org/apache/mina/core/service/AbstractIoServiceTest.java @@ -30,6 +30,7 @@ import org.apache.mina.filter.logging.LoggingFilter; import org.apache.mina.transport.socket.nio.NioSocketAcceptor; import org.apache.mina.transport.socket.nio.NioSocketConnector; +import org.apache.mina.util.AvailablePortFinder; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,16 +49,15 @@ */ public class AbstractIoServiceTest { - private static final int PORT = 9123; - @Test public void testDispose() throws IOException, InterruptedException { + while ( true) { List<String> threadsBefore = getThreadNames(); final IoAcceptor acceptor = new NioSocketAcceptor(); - acceptor.getFilterChain().addLast("logger", new LoggingFilter()); + // acceptor.getFilterChain().addLast("logger", new LoggingFilter()); acceptor.getFilterChain().addLast("codec", new ProtocolCodecFilter(new TextLineCodecFactory(Charset.forName("UTF-8")))); @@ -65,7 +65,8 @@ acceptor.getSessionConfig().setReadBufferSize(2048); acceptor.getSessionConfig().setIdleTime(IdleStatus.BOTH_IDLE, 10); - acceptor.bind(new InetSocketAddress(PORT)); + int nextAvailable = AvailablePortFinder.getNextAvailable(); + acceptor.bind(new InetSocketAddress(nextAvailable)); System.out.println("Server running ..."); final NioSocketConnector connector = new NioSocketConnector(); @@ -74,12 +75,12 @@ connector.setConnectTimeoutMillis(30 * 1000L); connector.setHandler(new ClientHandler()); - connector.getFilterChain().addLast("logger", new LoggingFilter()); + // connector.getFilterChain().addLast("logger", new LoggingFilter()); connector.getFilterChain().addLast("codec", new ProtocolCodecFilter(new TextLineCodecFactory(Charset.forName("UTF-8")))); // Start communication. - ConnectFuture cf = connector.connect(new InetSocketAddress("localhost", 9123)); + ConnectFuture cf = connector.connect(new InetSocketAddress("localhost", nextAvailable)); cf.awaitUninterruptibly(); IoSession session = cf.getSession(); @@ -103,7 +104,9 @@ public void operationComplete(IoFuture future) { System.out.println("managed session count=" + connector.getManagedSessionCount()); System.out.println("Disposing connector ..."); - connector.dispose(true); + // the doc states that the following should not be called with parameter TRUE from an IoFutureListener?! + // on the other hand, using FALSE does not work either + connector.dispose(false); System.out.println("Disposing connector ... *finished*"); } @@ -114,11 +117,11 @@ List<String> threadsAfter = getThreadNames(); - System.out.println("threadsBefore = " + threadsBefore); - System.out.println("threadsAfter = " + threadsAfter); + // System.out.println("threadsBefore = " + threadsBefore); + // System.out.println("threadsAfter = " + threadsAfter); // Assert.assertEquals(threadsBefore, threadsAfter); - + } } public static class ClientHandler extends IoHandlerAdapter { {code} > Leaking NioProcessors/NioSocketConnectors hanging in call to dispose > -------------------------------------------------------------------- > > Key: DIRMINA-1076 > URL: https://issues.apache.org/jira/browse/DIRMINA-1076 > Project: MINA > Issue Type: Bug > Affects Versions: 2.0.16 > Reporter: Christoph John > Priority: Major > Attachments: mina-dispose-hang.txt, mina-test-log.txt, > mina-test-patch.txt > > > Follow-up to mailing list discussion. > I was now able to reproduce the problem with a MINA test. Or let's say I did > the brute-force approach by re-running one test in an endless loop. > I have attached a patch of AbstractIoServiceTest (against > [https://github.com/apache/mina/tree/2.0]) and a stack trace. After a few > loops the test is stuck. You can see a lot of threads hanging in dispose() > and the test is stuck when it tries to dispose the acceptor. > > What is a little strange is that the javadoc says that > connector.dispose(TRUE) should not be called from an IoFutureListener, but in > the test it is done anyway. However, changing the parameter to FALSE does not > help either. > > Is there anything that can be done to prevent this hang? -- This message was sent by Atlassian JIRA (v7.6.3#76005)