Le 01/02/16 08:38, Emmanuel Lécharny a écrit :
> Le 01/02/16 00:51, Emmanuel Lécharny a écrit :
>> Hi guys,
>>
>> following the bugs Radovan found last week, I'm now reviewing the
>> session.close(true) execution, and I have found some new problems. I
>> just create a JIRA ofr one of them (DIRMINA-1025).
>>
>> Not that it's critical, but it's violating the contract : the session
>> should be closed immediately, but it's actually not.
>>
>> The thing is that it might be strange to call close() explicitely in the
>> IoHandler, but there are many reasons you might want to do that.
>> Typically, when a exceptionCaught event has been trapped, but it may
>> also because we have received a message that is wrong (or simply a
>> message that requires the session to be closed).
>>
>> As the process is purely asynchronous, we just change the status of the
>> session, and hope that every consecutive calls will check this status,
>> which is not the case :/ This has to be fixed. Typically, here is the
>> sequence of calls we can see :
>>
>> process()
>> process reads
>> ---> IoHandler.messageReceived()
>> // Do something
>> session.close()
>> <---
>> process writes
>> flush()
>> removeSessions()
>>
>> As you can see, we could either process some writes, of flush some
>> pending messages, before the session is removed.
>>
>> IMO, a calls to session.isClosing() should be added wherever we try to
>> do something on the session.
>>
>>
>> I have not yet analyzed the session( false ) call extensively, but I'm
>> quite sure some interesting things are going to be discovered ... Same
>> thing for the case where an exception is raised.
>>
> After a good night of sleep, I think that we should do two things :
> - deprecate the close(boolean) method, rename it closeNow() - standing
> for close(true)- and flushAndClose() - standing for close(false). The
> close() method will be remaped to flushAndClose(). That would clarify
> the purpose of teh close feature
> - when the closeNow() method is called, we should throw an exception
> (CloseException) that would be trapped in the IoProcessor loop, so that
> we can immediately kill the session, instead of potentially flushing
> some messages into the socket.
>
> wdyt ?
Side note : flushAndClose() is not the right name : we already have a
closeOnFlush() that is much better and do the job.
