> On 05/05/2021 18:28 Ryan Beethe <r...@splintermail.com> wrote: > > > On Wed, May 05, 2021 at 10:53:30AM +0300, Aki Tuomi wrote: > > > > > On 04/05/2021 16:42 Ryan Beethe <r...@splintermail.com> wrote: > > > > > > > > > On Mon, May 03, 2021 at 09:14:13AM +0300, Aki Tuomi wrote: > > > > > > > > > On 01/05/2021 18:32 Ryan Beethe <r...@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
1-3 . IDLE is sometimes implemented by *imap-hibernate* process, which is a different process, so the connection is *moved* there (removed totally from imap process, imap process dies, and connection continues to live in imap-hibernate process). IDLE command is not the best to look for ideas, as it's pretty complicated thing. You might want to check https://github.com/freswa/dovecot-xaps-plugin for ideas. Aki
