Hi Frédéric, Sorry for extremely late response. Comments inline...
Frédéric Brégier wrote:
> I have re-read the source code and I found some
> bad reading from my first thought.
>
> First in messageReceived, it only goes to the
> next filter only if it is not a keep alive message,
> so it is perfectly OK.
>
> In sessionIdle, it really passes every idle status
> to the next filter, whatever the KeepAliveFilter does.
> I think this should not be. I believe it should go
> to the next filter only if it was not catch by the K-A-filter.
>
> Now on the logic of this filter, I believe it is perfectly ok for
> a ping-pong behaviour, but not in "Deaf Speaker" mode.
> Indeed in Deaf Speaker mode, it only sends a ping message
> and never receives a pong so the READER_IDLE is not
> clear, but it should since I feel like the spirit of this filter
> is to clear it.
>
> Also what about the WRITER_IDLE or BOTH_IDLE ?
> This filter will never catch it...
>
> For the "next filter" case, I would suggest the following (I have not tested
> it though...)
> in sessionIdle
> public void sessionIdle(
> NextFilter nextFilter, IoSession session, IdleStatus status) throws
> Exception {
> try {
> ...
> } finally {
> if (status != IdleStatus.READER_IDLE) {// only if not READER_IDLE
> nextFilter.sessionIdle(session, status);
> }
> }
> }
You are right. KeepAliveFilter should not propagate
sessionIdle(READER_IDLE) event. I've checked in the fix and the
following is the URL of the related JIRA issue:
https://issues.apache.org/jira/browse/DIRMINA-562
> For BOTH_IDLE, does the IdleStatus contains READ_IDLE ?
> If not, should all tests about READ_IDLE be with BOTH_IDLE ?
If sessionIdle(BOTH_IDLE) is fired, sessionIdle(READR_IDLE) and
sessionIdle(WRITER_IDLE) are fired together.
> For WRITE_IDLE, I don't know if it is relevant.
> If so, then I would suggest to make two kind of filter, one for READ_IDLE
> and another one for WRITER_IDLE, so the user can choose one or the other
> or both.
I'm not sure what we can do with WRITER_IDLE for sending keep-alive
messages. Could you give me some scenarios?
> Now on the "Deaf Speaker" mode issue, I don't know how in Mina
> we can "clear" the Idle (in this case the READ_IDLE) ?
> So I would suggest, if it is not possible, to say in the API doc something
> about this limitation (if there is one of course, I can be completely wrong
> ;-)
It was a documentation error. You have to set the keep alive policy
additionally to 'OFF' to use the deaf speaker mode:
https://issues.apache.org/jira/browse/DIRMINA-563
HTH,
--
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/
signature.asc
Description: OpenPGP digital signature
