[RFC PATCH v2] imapd.c: imapoptions: implement idle timeout

[ellie timoney]

On Tue, Sep 20, 2016, at 08:37 PM, Robert Mueller via Cyrus-devel wrote:
> 
> > The thing is, during IDLE, we're not blocked on a read, we're blocked on
> > a select (in idle_wait()).  We don't even try to read from the client
> > until the select tells us it's readable. If the client has dropped out,
> > select never tells us it's readable, so we never try to read, so the
> > timeout doesn't get checked, so we stay there indefinitely.
> 
> No, the select() call itself has a timeout, and idle_wait() will return
> after that timeout gets hit.

But the select's timeout (imapidlepoll) is how long it should wait for
anything to happen before saying "nothing happened".  When select times
out, idle_wait() returns (exactly) the flags IDLE_MAILBOX|IDLE_ALERT.

Back in imapd.c, cmd_idle() only reads from the client socket if
IDLE_INPUT is flagged.  Which it's not after a select timeout, so if
tcp_keepalive has detected a connection dropout, cyrus still doesn't
know about it.

When IDLE_MAILBOX is flagged, which in this case it is, we call into
index_check() to notify about mailbox activity.  But if there has not
been any activity on the mailbox, we won't try to write anything to the
client socket, and so we can't see that the connection has dropped out
here either.

When IDLE_ALERT is flagged, we check whether we need to exit (shutdown
file and/or userid is now in userdeny) and do so -- unrelated.

So it still looks to me like a client idling on dormant mailbox can drop
out, and even with tcp_keepalive, cyrus won't notice.

> > timeout.  Comms between idled and imapd were handled with signals.
> 
> Yes, and it was buggy and evil. The pipe system that replaced it is much
> saner.

Yeah agreed