From fa2f234343eb4e2a76288a17cfdb4ae6306579f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emysl=20Janouch?= Date: Sun, 10 Aug 2014 06:18:32 +0200 Subject: [PATCH] ZyklonB: cleanup not only wrt. timers The code isn't async enough and needs some further changes. --- zyklonb.c | 180 +++++++++++++++++++++++++++++------------------------- 1 file changed, 97 insertions(+), 83 deletions(-) diff --git a/zyklonb.c b/zyklonb.c index d17bf7c..10df9b0 100644 --- a/zyklonb.c +++ b/zyklonb.c @@ -108,6 +108,8 @@ struct bot_context { struct str_map config; ///< User configuration regex_t *admin_re; ///< Regex to match our administrator + bool reconnect; ///< Whether to reconnect on conn. fail. + unsigned long reconnect_delay; ///< Reconnect delay in seconds int irc_fd; ///< Socket FD of the server struct str read_buffer; ///< Input yet to be processed @@ -178,6 +180,7 @@ bot_context_free (struct bot_context *self) static void irc_shutdown (struct bot_context *ctx) { + // TODO: set a timer after which we cut the connection? // Generally non-critical if (ctx->ssl) soft_assert (SSL_shutdown (ctx->ssl) != -1); @@ -185,22 +188,31 @@ irc_shutdown (struct bot_context *ctx) soft_assert (shutdown (ctx->irc_fd, SHUT_WR) == 0); } -static void -initiate_quit (struct bot_context *ctx) -{ - irc_shutdown (ctx); - ctx->quitting = true; -} - static void try_finish_quit (struct bot_context *ctx) { - if (!ctx->quitting) - return; - if (ctx->irc_fd == -1 && !ctx->plugins) + if (ctx->quitting && ctx->irc_fd == -1 && !ctx->plugins) ctx->polling = false; } +static bool plugin_zombify (struct plugin_data *); + +static void +initiate_quit (struct bot_context *ctx) +{ + // Initiate bringing down of the two things that block our shutdown: + // a/ the IRC socket, b/ our child processes: + + for (struct plugin_data *plugin = ctx->plugins; + plugin; plugin = plugin->next) + plugin_zombify (plugin); + if (ctx->irc_fd != -1) + irc_shutdown (ctx); + + ctx->quitting = true; + try_finish_quit (ctx); +} + static bool irc_send (struct bot_context *ctx, const char *format, ...) ATTRIBUTE_PRINTF (2, 3); @@ -649,6 +661,8 @@ plugin_zombify (struct plugin_data *plugin) // remaining commands it attempts to send to us before it finally dies. str_map_set (&plugin->ctx->plugins_by_name, plugin->name, NULL); plugin->is_zombie = true; + + // TODO: wait a few seconds and then send SIGKILL to the plugin return true; } @@ -1016,8 +1030,6 @@ plugin_unload (struct bot_context *ctx, const char *name, struct error **e) // TODO: add a `kill zombies' command to forcefully get rid of processes // that do not understand the request. - // TODO: set a timeout before we go for a kill automatically (and if this - // was a reload request, try to bring the plugin back up) return true; } @@ -1162,6 +1174,9 @@ static void process_plugin_reload (struct bot_context *ctx, const struct irc_message *msg, const char *name) { + // XXX: we might want to wait until the plugin terminates before we try + // to reload it (so that it can save its configuration or whatever) + // So far the only error that can occur is that the plugin hasn't been // loaded, which in this case doesn't really matter. plugin_unload (ctx, name, NULL); @@ -1380,60 +1395,49 @@ start: return IRC_READ_ERROR; } -static bool irc_connect (struct bot_context *ctx, struct error **e); +static bool irc_connect (struct bot_context *, struct error **); +static void irc_queue_reconnect (struct bot_context *); static void -irc_try_reconnect (struct bot_context *ctx) +irc_cancel_timers (struct bot_context *ctx) { - if (!soft_assert (ctx->irc_fd == -1)) - return; + ssize_t i; + struct poller_timers *timers = &ctx->poller.timers; + while ((i = poller_timers_find_by_data (timers, ctx)) != -1) + poller_timers_remove_at_index (timers, i); +} - const char *reconnect_str = str_map_find (&ctx->config, "reconnect"); - hard_assert (reconnect_str != NULL); // We have a default value for this +static void +irc_on_reconnect_timeout (void *user_data) +{ + struct bot_context *ctx = user_data; - bool reconnect; - if (!set_boolean_if_valid (&reconnect, reconnect_str)) + struct error *e = NULL; + if (irc_connect (ctx, &e)) { - print_fatal ("invalid configuration value for `%s'", "recover"); - try_finish_quit (ctx); + // TODO: inform plugins about the new connection return; } - if (!reconnect) - return; - const char *delay_str = str_map_find (&ctx->config, "reconnect_delay"); - hard_assert (delay_str != NULL); // We have a default value for this + print_error ("%s", e->message); + error_free (e); + irc_queue_reconnect (ctx); +} - unsigned long delay; - if (!xstrtoul (&delay, delay_str, 10)) - { - print_error ("invalid configuration value for `%s'", - "reconnect_delay"); - delay = 0; - } - - while (true) - { - // TODO: this would be better suited by a timeout event; - // remember to update try_finish_quit() etc. to reflect this - print_status ("trying to reconnect in %ld seconds...", delay); - sleep (delay); - - struct error *e = NULL; - if (irc_connect (ctx, &e)) - break; - - print_error ("%s", e->message); - error_free (e); - } - - // TODO: inform plugins about the new connection +static void +irc_queue_reconnect (struct bot_context *ctx) +{ + hard_assert (ctx->irc_fd == -1); + print_status ("trying to reconnect in %ld seconds...", + ctx->reconnect_delay); + poller_timers_add (&ctx->poller.timers, + irc_on_reconnect_timeout, ctx, ctx->reconnect_delay * 1000); } static void on_irc_disconnected (struct bot_context *ctx) { - // Get rid of the dead socket etc. + // Get rid of the dead socket and related things if (ctx->ssl) { SSL_free (ctx->ssl); @@ -1452,19 +1456,15 @@ on_irc_disconnected (struct bot_context *ctx) // TODO: inform plugins about the disconnect event + // All of our timers have lost their meaning now + irc_cancel_timers (ctx); + if (ctx->quitting) - { - // Unload all plugins - // TODO: wait for a few seconds and then send SIGKILL to all plugins - for (struct plugin_data *plugin = ctx->plugins; - plugin; plugin = plugin->next) - plugin_zombify (plugin); - try_finish_quit (ctx); - return; - } - - irc_try_reconnect (ctx); + else if (!ctx->reconnect) + initiate_quit (ctx); + else + irc_queue_reconnect (ctx); } static void @@ -1484,13 +1484,9 @@ on_irc_timeout (void *user_data) } static void -irc_reset_timeouts (struct bot_context *ctx) +irc_reset_connection_timeouts (struct bot_context *ctx) { - ssize_t i; - struct poller_timers *timers = &ctx->poller.timers; - while ((i = poller_timers_find_by_data (timers, ctx)) != -1) - poller_timers_remove_at_index (timers, i); - + irc_cancel_timers (ctx); poller_timers_add (&ctx->poller.timers, on_irc_timeout, ctx, 3 * 60 * 1000); poller_timers_add (&ctx->poller.timers, @@ -1544,7 +1540,7 @@ end: if (disconnected) on_irc_disconnected (ctx); else - irc_reset_timeouts (ctx); + irc_reset_connection_timeouts (ctx); } static bool @@ -1585,7 +1581,7 @@ irc_connect (struct bot_context *ctx, struct error **e) // 3/ /* O_CLOEXEC */ But only if the QUIT message proves unreliable. poller_set (&ctx->poller, ctx->irc_fd, POLLIN, (poller_dispatcher_func) on_irc_readable, ctx); - irc_reset_timeouts (ctx); + irc_reset_connection_timeouts (ctx); irc_send (ctx, "NICK %s", nickname); irc_send (ctx, "USER %s 8 * :%s", username, realname); @@ -1593,22 +1589,38 @@ irc_connect (struct bot_context *ctx, struct error **e) } static bool -load_admin_regex (struct bot_context *ctx) +parse_config (struct bot_context *ctx, struct error **e) { + const char *reconnect_str = str_map_find (&ctx->config, "reconnect"); + hard_assert (reconnect_str != NULL); // We have a default value for this + if (!set_boolean_if_valid (&ctx->reconnect, reconnect_str)) + { + error_set (e, "invalid configuration value for `%s'", "reconnect"); + return false; + } + + const char *delay_str = str_map_find (&ctx->config, "reconnect_delay"); + hard_assert (delay_str != NULL); // We have a default value for this + if (!xstrtoul (&ctx->reconnect_delay, delay_str, 10)) + { + error_set (e, "invalid configuration value for `%s'", + "reconnect_delay"); + return false; + } + hard_assert (!ctx->admin_re); const char *admin = str_map_find (&ctx->config, "admin"); - if (!admin) return true; - struct error *e = NULL; - ctx->admin_re = regex_compile (admin, REG_EXTENDED | REG_NOSUB, &e); - if (!e) + struct error *error = NULL; + ctx->admin_re = regex_compile (admin, REG_EXTENDED | REG_NOSUB, &error); + if (!error) return true; - print_error ("invalid configuration value for `%s': %s", - "admin", e->message); - error_free (e); + error_set (e, "invalid configuration value for `%s': %s", + "admin", error->message); + error_free (error); return false; } @@ -1618,10 +1630,13 @@ on_signal_pipe_readable (const struct pollfd *fd, struct bot_context *ctx) char *dummy; (void) read (fd->fd, &dummy, 1); - // XXX: do we need to check if we have a connection? if (g_termination_requested && !ctx->quitting) { - irc_send (ctx, "QUIT :Terminated by signal"); + // There may be a timer set to reconnect to the server + irc_cancel_timers (ctx); + + if (ctx->irc_fd != -1) + irc_send (ctx, "QUIT :Terminated by signal"); initiate_quit (ctx); } @@ -1769,9 +1784,8 @@ main (int argc, char *argv[]) (poller_dispatcher_func) on_signal_pipe_readable, &ctx); plugin_load_all_from_config (&ctx); - if (!load_admin_regex (&ctx)) - exit (EXIT_FAILURE); - if (!irc_connect (&ctx, &e)) + if (!parse_config (&ctx, &e) + || !irc_connect (&ctx, &e)) { print_error ("%s", e->message); error_free (e);