From fea09a415cf89223d8b17c5982257c9b3c357cbc Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Wed, 3 Feb 2021 20:42:50 +0200 Subject: [PATCH] lib: io_loop_context_new() no longer implicitly activates the context The new behavior requires explicitly activating the context. This change was needed, because an implicit creation activated the context but didn't call any of the callbacks. If ioloop wasn't run, the activation callbacks were never called. This meant that when context was deactivated, the deactivation callbacks weren't run either. --- src/imap-hibernate/imap-client.c | 2 +- src/lib-storage/mail-storage-service.c | 1 + src/lib/ioloop.c | 23 ++++++++++++++--------- src/lib/ioloop.h | 20 ++++++++++++-------- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/imap-hibernate/imap-client.c b/src/imap-hibernate/imap-client.c index 358030f887..d62a346beb 100644 --- a/src/imap-hibernate/imap-client.c +++ b/src/imap-hibernate/imap-client.c @@ -730,7 +730,7 @@ void imap_client_create_finish(struct imap_client *client) io_loop_context_add_callbacks(client->ioloop_ctx, imap_client_io_activate_user, imap_client_io_deactivate_user, client); - imap_client_io_activate_user(client); + io_loop_context_switch(client->ioloop_ctx); if (client->state.idle_cmd) { client->io = io_add(client->fd, IO_READ, diff --git a/src/lib-storage/mail-storage-service.c b/src/lib-storage/mail-storage-service.c index 8f96fb1ae7..e3fdd17af7 100644 --- a/src/lib-storage/mail-storage-service.c +++ b/src/lib-storage/mail-storage-service.c @@ -1526,6 +1526,7 @@ mail_storage_service_next_real(struct mail_storage_service_ctx *ctx, mail_storage_service_io_deactivate_user_cb, user); } + io_loop_context_switch(user->ioloop_ctx); if ((user->flags & MAIL_STORAGE_SERVICE_FLAG_NO_RESTRICT_ACCESS) == 0) { if (service_drop_privileges(user, &priv, diff --git a/src/lib/ioloop.c b/src/lib/ioloop.c index 00b56cb982..1e854c534e 100644 --- a/src/lib/ioloop.c +++ b/src/lib/ioloop.c @@ -727,7 +727,7 @@ void io_loop_run(struct ioloop *ioloop) io_loop_initialize_handler(ioloop); if (ioloop->cur_ctx != NULL) - io_loop_context_unref(&ioloop->cur_ctx); + io_loop_context_deactivate(ioloop->cur_ctx); /* recursive io_loop_run() isn't allowed for the same ioloop. it can break backends. */ @@ -1025,14 +1025,6 @@ struct ioloop_context *io_loop_context_new(struct ioloop *ioloop) ctx->refcount = 2; ctx->ioloop = ioloop; i_array_init(&ctx->callbacks, 4); - - if (ioloop->cur_ctx != NULL) { - io_loop_context_deactivate(ioloop->cur_ctx); - /* deactivation may remove the cur_ctx */ - if (ioloop->cur_ctx != NULL) - io_loop_context_unref(&ioloop->cur_ctx); - } - ioloop->cur_ctx = ctx; return ctx; } @@ -1150,6 +1142,19 @@ void io_loop_context_deactivate(struct ioloop_context *ctx) io_loop_context_unref(&ctx); } +void io_loop_context_switch(struct ioloop_context *ctx) +{ + if (ctx->ioloop->cur_ctx != NULL) { + if (ctx->ioloop->cur_ctx == ctx) + return; + io_loop_context_deactivate(ctx->ioloop->cur_ctx); + /* deactivation may remove the cur_ctx */ + if (ctx->ioloop->cur_ctx != NULL) + io_loop_context_unref(&ctx->ioloop->cur_ctx); + } + io_loop_context_activate(ctx); +} + struct ioloop_context *io_loop_get_current_context(struct ioloop *ioloop) { return ioloop->cur_ctx; diff --git a/src/lib/ioloop.h b/src/lib/ioloop.h index 901686e4fb..75c6ef9f87 100644 --- a/src/lib/ioloop.h +++ b/src/lib/ioloop.h @@ -215,14 +215,15 @@ void io_loop_remove_switch_callback(io_switch_callback_t *callback); void io_loop_add_destroy_callback(io_destroy_callback_t *callback); void io_loop_remove_destroy_callback(io_destroy_callback_t *callback); -/* Create a new ioloop context. This context is automatically attached to all - the following I/Os and timeouts that are added until the context is - deactivated (e.g. returning to back to a running ioloop). Whenever such - added I/O or timeout callback is called, this context is automatically - activated. - - Creating this context already deactivates any currently running context - and activates the newly created context. */ +/* Create a new ioloop context. While the context is activated, it's + automatically attached to all the following I/Os and timeouts that are + added until the context is deactivated (e.g. returning to back to a running + ioloop). Whenever such added I/O or timeout callback is called, this context + is automatically activated. + + After the context is created, callbacks should be added to it and the + context should be activated with either io_loop_context_activate() or + io_loop_context_switch(). */ struct ioloop_context *io_loop_context_new(struct ioloop *ioloop); void io_loop_context_ref(struct ioloop_context *ctx); void io_loop_context_unref(struct ioloop_context **ctx); @@ -266,6 +267,9 @@ void io_loop_context_activate(struct ioloop_context *ctx); active or it assert-crashes. This should be called only after a context was explicitly activated with io_loop_context_activate(). */ void io_loop_context_deactivate(struct ioloop_context *ctx); +/* If there's an active ioloop context, deactivate it. Then activate the given + context. Usually used after creating a new context. */ +void io_loop_context_switch(struct ioloop_context *ctx); /* Returns fd, which contains all of the ioloop's current notifications. When it becomes readable, there is a new notification. Calling this function -- 2.47.3