On Wed, May 05, 2021 at 10:53:30AM +0300, Aki Tuomi
wrote:>
> > On 04/05/2021 16:42 Ryan Beethe <ryan at splintermail.com>
wrote:
> >
> >
> > On Mon, May 03, 2021 at 09:14:13AM +0300, Aki Tuomi wrote:
> > >
> > > > On 01/05/2021 18:32 Ryan Beethe <ryan at
splintermail.com> wrote:
> > > >
> > > > 1. Why does cmd-idle.c sometimes call client_command_free()?
But
> > > > sometimes it doesn't?
> > > >
> > > > For example, cmd_idle_continue() frees it in some
branches but not
> > > > others. That makes no sense to me; it seems like it
should be based
> > > > on your entrypoint (mailbox notify callback vs input
callback vs
> > > > timeout callback), not based on which branch of logic
within that
> > > > entrypoint.
> > > >
> > > > 2. Why does cmd-idle.c ever call client_destroy()? That
seems like
> > > > something that should be invoked only by the imap process,
not by any
> > > > command.
> > > >
> > > > It calls it in cmd-idle.c:idle_callback (which is a
mailbox notify
> > > > callback). It invokes it after idle_sync_now() when it
detects that
> > > > client->disconnected is set. Maybe that happens in
imap_sync_init()
> > > > or something?
> > > >
> > > > 3. Why does cmd-idle.c ever call client_disconnect()? That
also seems
> > > > like the responsibility of the imap process, and not any
command.
> > > >
> > > > idle_client_input_more() detects when i_stream_read
returns -1,
> > > > meaning that the client has *already disconnected*.
Then it calls
> > > > client_disconnect().
> > > >
> > > > I think this is the crazy part... the istream is
effectively unique
> > > > to the imap process, so it seems unreasonable that any
command is
> > > > responsible for cleaning it up; it should just always
happen at the
> > > > imap process level before exiting, right?
> > > >
> > >
> > > IDLE cmd can be sometimes delegated to a separate worker called
imap-hibernate, in which case the connection is moved to another process. This
explains about all your questions.
> >
> > Wait, but then why does APPEND also make each of these calls? APPEND
> > can't be hibernated, as far as I can tell?
> >
>
> Because APPEND might need to read quite a lot of data from the client.
So then I am back at my original questions. Maybe I can guess at some
answers and you can tell me if I'm understanding correctly:
1. Why does cmd-idle.c sometimes call client_command_free()? But
sometimes it doesn't?
Earlier I said I though cmd_idle_continue() freed it in some
branches but not others, but I think I was mistaken. It looks
like the only path where client_command_free is called is inside
an io_add_istream callback. That makes sense, and I can do the
same thing with my command.
2. Why does cmd-idle.c ever call client_destroy()? That seems like
something that should be invoked only by the imap process, not by
any command.
This is only ever triggered by idle_callback, which is a
mailbox_notify_changes callback, which I don't have to interact
with, so maybe I can ignore this.
3. Why does cmd-idle.c ever call client_disconnect()? That also
seems like the responsibility of the imap process, and not any
command.
While I'm still not sure why the imap process is not responsible
for calling this, it does seem like it only gets called when
i_stream_read() returns -1, and I can probably immitate that
without much risk.
But wait, why does cmd-idle.c call client_disconnect() when
i_stream_read() returns -1, but cmd-append.c calls
client_command_free() and client_destroy() but not
client_disconnect()?
> > > You probably shold look some much more simple commands as
> > > insipiration. Try looking e.g. how cmd_id is implemented instead.
> >
> > I implemented a simpler command as well, but because it was simple I
> > didn't have any questions :)
> >
> > Unfortunately I do need a long-running command more like IDLE as well.
>
> What kind of "long running command" did you have in mind?
My email service offers a layer of encryption which is not transparent
to IMAP, and where the keys are created and kept on each client device.
Since IMAP synchronization is bidirectional, each client needs to
encrypt uploaded messages to all known client devices. Thus, clients
need a way update their list of all known keys.
So the command is roughly:
tag XKEYSYNC [known_fingerprint ...]
...
DONE
And the responses are rougly:
* XKEYSYNC DELETED fingerprint
* XKEYSYNC CREATED public_key
The full source can be found at:
github.com/splintermail/splintermail-client/blob/dev/server/xkeysync.c
Ryan