ZyklonB: cleanup not only wrt. timers

The code isn't async enough and needs some further changes.

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)
+	if (ctx->quitting && ctx->irc_fd == -1 && !ctx->plugins)
-		return;
-	if (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))
+	ssize_t i;
-		return;
+	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");
+static void
-	hard_assert (reconnect_str != NULL);  // We have a default value for this
+irc_on_reconnect_timeout (void *user_data)
-
+{
-	bool reconnect;
+	struct bot_context *ctx = user_data;
-	if (!set_boolean_if_valid (&reconnect, reconnect_str))
-	{
-		print_fatal ("invalid configuration value for `%s'", "recover");
-		try_finish_quit (ctx);
-		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
-
-	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;
+	{
+		// TODO: inform plugins about the new connection
+		return;
+	}
 
 	print_error ("%s", e->message);
 	error_free (e);
-	}
+	irc_queue_reconnect (ctx);
+}
 
-	// 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;
+	else if (!ctx->reconnect)
-	}
+		initiate_quit (ctx);
-
+	else
-	irc_try_reconnect (ctx);
+		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;
+	irc_cancel_timers (ctx);
-	struct poller_timers *timers = &ctx->poller.timers;
-	while ((i = poller_timers_find_by_data (timers, ctx)) != -1)
-		poller_timers_remove_at_index (timers, i);
-
 	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;
+	struct error *error = NULL;
-	ctx->admin_re = regex_compile (admin, REG_EXTENDED | REG_NOSUB, &e);
+	ctx->admin_re = regex_compile (admin, REG_EXTENDED | REG_NOSUB, &error);
-	if (!e)
+	if (!error)
 		return true;
 
-	print_error ("invalid configuration value for `%s': %s",
+	error_set (e, "invalid configuration value for `%s': %s",
-		"admin", e->message);
+		"admin", error->message);
-	error_free (e);
+	error_free (error);
 	return false;
 }
 
@@ -1618,9 +1630,12 @@ 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)
 	{
+		// 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))
+	if (!parse_config (&ctx, &e)
-		exit (EXIT_FAILURE);
+	 || !irc_connect (&ctx, &e))
-	if (!irc_connect (&ctx, &e))
 	{
 		print_error ("%s", e->message);
 		error_free (e);