[ 
https://issues.apache.org/jira/browse/DIRMINA-1086?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16502951#comment-16502951
 ] 

Emmanuel Lecharny commented on DIRMINA-1086:
--------------------------------------------

Thanks for the feedback ! Glad to see that the pb was due to the GC not coping 
with the load.

Regarding the {{SelectionKey}} invalidation, the idea would be to cancel it 
when a {{closeNow}} is called. That was what I was looking for yesterday 
evening. The problem being this method is executed in the {{AbstractIoSession}} 
class, which has no knowledge about the {{SelectionKey}}, otherwise it would 
have been quite natural to get this key, and cancel it.

Actually, this is what happens, but after a looooong set of calls, and only 
after the {{IoProcessor}} in charge for this session has been kicked and calls 
the {{removeSessions}} method, which calls the {{removeNow}} method :

{code:java}
        private boolean removeNow(S session) {
            clearWriteRequestQueue(session);

            try {
                destroy(session);
...
{code}

and in {{NioSession}} :

{code:java}
    protected void destroy(NioSession session) throws Exception {
        ByteChannel ch = session.getChannel();
        
        SelectionKey key = session.getSelectionKey();
        
        if (key != null) {
            key.cancel();
        }
        
        if ( ch.isOpen() ) {
            ch.close();
        }
    }
{code}

At this point, a {{getState(session)}} will return {{CLOSING}}, but we don't 
really care.

Note that I don't see and drawback in modifying the {{Processor.removeNow(S 
session)}} method to handle the {{OPENED}} and {{CLOSING}} state the exact same 
way :

{code:java}
        private int removeSessions() {
            int removedSessions = 0;

            for (S session = removingSessions.poll(); session != null; session 
= removingSessions.poll()) {
                SessionState state = getState(session);

                // Now deal with the removal accordingly to the session's state
                switch (state) {
                case OPENED:
                case CLOSING:
                    // Try to remove this session
                    if (removeNow(session)) {
                        removedSessions++;
                    }

                    break;
                    ...
{code}

That would cover the case where the channel has been closed or the 
{{SelectionKey}} cancelled by another call (still have to check if it's a 
remote possibility). 

Jonathan, some thoughts ?


> IoSessions closed by filterChain.fireExceptionCaught(e) are kept in memory
> --------------------------------------------------------------------------
>
>                 Key: DIRMINA-1086
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-1086
>             Project: MINA
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.0.17
>         Environment: mina in ApacheDS
>            Reporter: Wenxiang Qiu
>            Priority: Major
>
> I'm using ApacheDS, I found this problem and am not sure if it's a mina issue 
> or an ApacheDS issue.
>  
> AbstractPollingIoProcessor#read looks like this:
>  
> {code:java}
> private void read(S session) {
>     IoSessionConfig config = session.getConfig();
>     int bufferSize = config.getReadBufferSize();
>     IoBuffer buf = IoBuffer.allocate(bufferSize);
>     final boolean hasFragmentation = 
> session.getTransportMetadata().hasFragmentation();
>     try {
>         /* omitted */
>         if (ret < 0) {
>             IoFilterChain filterChain = session.getFilterChain();
>             filterChain.fireInputClosed();
>         }
>     } catch (Exception e) {
>         if ((e instanceof IOException) &&
> (!(e instanceof PortUnreachableException)
> || !AbstractDatagramSessionConfig.class.isAssignableFrom(config.getClass())
> || ((AbstractDatagramSessionConfig) config).isCloseOnPortUnreachable())) {
>             scheduleRemove(session);
>         }
>         IoFilterChain filterChain = session.getFilterChain();
>         filterChain.fireExceptionCaught(e);
>     }
> }
> {code}
>  
> When an exception occurs with an LDAP connection, e.g. Connection reset by 
> peer, catch block is entered and ExcpetionCaught gets fired:
> {code:java}
> filterChain.fireExceptionCaught(e);{code}
> The session is thus closed by LdapProtocolHandler#exceptionCaught:
> {code:java}
> public void exceptionCaught( IoSession session, Throwable cause )
> {
>     if ( cause.getCause() instanceof ResponseCarryingMessageException )
>     {
>         ResponseCarryingMessageException rcme = ( 
> ResponseCarryingMessageException ) cause.getCause();
>         if ( rcme.getResponse() != null )
>         {
>             session.write( rcme.getResponse() );
>             return;
>         }
>     }
>     LOG.warn( "Unexpected exception forcing session to close: sending 
> disconnect notice to client.", cause );
>     session.write( NoticeOfDisconnect.PROTOCOLERROR );
>     LdapSession ldapSession = 
> this.ldapServer.getLdapSessionManager().removeLdapSession( session );
>     cleanUpSession( ldapSession );
>     session.close( true );
> }
> {code}
> Although this session is scheduled for removal, due to its state being 
> closing, AbstractPollingIoProcessor#removeSessions does nothing about this 
> session:
> {code:java}
> case CLOSING:
>     // Skip if channel is already closed
>     // In any case, remove the session from the queue
>     removedSessions++;
>     break;
> {code}
> Consequence is that this session is kept forever in 
> IoServiceListenerSupport.managedSessions, and as its size grows, this 
> ConcurrentMap can take up quite a large amount of memory.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to