From 43d3cc3d8a4001c9cb65871a651fe81cbc6ee98e Mon Sep 17 00:00:00 2001 From: Mikhail Galanin Date: Thu, 30 Aug 2018 14:03:01 +0100 Subject: [PATCH] [Minor] Log should not depend on config When config is being closed, some destructors could be called, and that dtors could write into log. Hence, it is better to terminate config and only then close log. And log should not refer disposed config --- src/controller.c | 2 +- src/fuzzy_storage.c | 4 +-- src/libserver/worker_util.c | 4 +-- src/libutil/logger.c | 54 +++++++++++++++++++++++++++---------- src/libutil/logger.h | 4 +-- src/log_helper.c | 2 +- src/lua_worker.c | 3 ++- src/rspamadm/rspamadm.c | 2 +- src/rspamd.c | 8 +++--- src/rspamd_proxy.c | 3 +-- src/worker.c | 2 +- 11 files changed, 58 insertions(+), 30 deletions(-) diff --git a/src/controller.c b/src/controller.c index 72220aa3c..6cb5eaa62 100644 --- a/src/controller.c +++ b/src/controller.c @@ -3855,7 +3855,6 @@ start_controller_worker (struct rspamd_worker *worker) rspamd_stat_close (); rspamd_http_router_free (ctx->http); - rspamd_log_close (worker->srv->logger); if (ctx->cached_password.len > 0) { m = (gpointer)ctx->cached_password.begin; @@ -3870,6 +3869,7 @@ start_controller_worker (struct rspamd_worker *worker) g_hash_table_unref (ctx->plugins); g_hash_table_unref (ctx->custom_commands); REF_RELEASE (ctx->cfg); + rspamd_log_close (worker->srv->logger, TRUE); exit (EXIT_SUCCESS); } diff --git a/src/fuzzy_storage.c b/src/fuzzy_storage.c index cd0266844..0636aeff7 100644 --- a/src/fuzzy_storage.c +++ b/src/fuzzy_storage.c @@ -3017,8 +3017,6 @@ start_fuzzy (struct rspamd_worker *worker) g_array_free (ctx->updates_pending, TRUE); } - rspamd_log_close (worker->srv->logger); - if (ctx->peer_fd != -1) { if (worker->index == 0) { event_del (&ctx->peer_ev); @@ -3032,5 +3030,7 @@ start_fuzzy (struct rspamd_worker *worker) REF_RELEASE (ctx->cfg); + rspamd_log_close (worker->srv->logger, TRUE); + exit (EXIT_SUCCESS); } diff --git a/src/libserver/worker_util.c b/src/libserver/worker_util.c index 7a93a4304..64e0bae2e 100644 --- a/src/libserver/worker_util.c +++ b/src/libserver/worker_util.c @@ -647,7 +647,7 @@ rspamd_fork_worker (struct rspamd_main *rspamd_main, } /* Do silent log reopen to avoid collisions */ - rspamd_log_close (rspamd_main->logger); + rspamd_log_close (rspamd_main->logger, FALSE); if (rspamd_main->cfg->log_silent_workers) { @@ -771,7 +771,7 @@ rspamd_hard_terminate (struct rspamd_main *rspamd_main) msg_err_main ("shutting down Rspamd due to fatal error"); - rspamd_log_close (rspamd_main->logger); + rspamd_log_close (rspamd_main->logger, TRUE); exit (EXIT_FAILURE); } diff --git a/src/libutil/logger.c b/src/libutil/logger.c index 0e20ec8e0..027c21da1 100644 --- a/src/libutil/logger.c +++ b/src/libutil/logger.c @@ -69,7 +69,14 @@ struct rspamd_logger_error_log { */ struct rspamd_logger_s { rspamd_log_func_t log_func; - struct rspamd_config *cfg; + enum rspamd_log_type log_type; + gint log_facility; + gint log_level; + gchar *log_file; + gboolean log_buffered; + gboolean log_silent_workers; + guint32 log_buf_size; + struct rspamd_logger_error_log *errlog; struct rspamd_cryptobox_pubkey *pk; struct rspamd_cryptobox_keypair *keypair; @@ -249,7 +256,7 @@ gint rspamd_log_open_priv (rspamd_logger_t *rspamd_log, uid_t uid, gid_t gid) { if (!rspamd_log->opened) { - switch (rspamd_log->cfg->log_type) { + switch (rspamd_log->log_type) { case RSPAMD_LOG_CONSOLE: /* Do nothing with console */ rspamd_log->fd = -1; @@ -257,23 +264,23 @@ rspamd_log_open_priv (rspamd_logger_t *rspamd_log, uid_t uid, gid_t gid) case RSPAMD_LOG_SYSLOG: #ifdef HAVE_SYSLOG_H openlog ("rspamd", LOG_NDELAY | LOG_PID, - rspamd_log->cfg->log_facility); + rspamd_log->log_facility); #endif break; case RSPAMD_LOG_FILE: - rspamd_log->fd = open (rspamd_log->cfg->log_file, + rspamd_log->fd = open (rspamd_log->log_file, O_CREAT | O_WRONLY | O_APPEND, S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH); if (rspamd_log->fd == -1) { fprintf (stderr, - "open_log: cannot open desired log file: %s, %s", - rspamd_log->cfg->log_file, strerror (errno)); + "open_log: cannot open desired log file: %s, %s my pid: %d", + rspamd_log->log_file, strerror (errno), getpid ()); return -1; } if (fchown (rspamd_log->fd, uid, gid) == -1) { fprintf (stderr, "open_log: cannot chown desired log file: %s, %s", - rspamd_log->cfg->log_file, strerror (errno)); + rspamd_log->log_file, strerror (errno)); close (rspamd_log->fd); return -1; } @@ -290,7 +297,7 @@ rspamd_log_open_priv (rspamd_logger_t *rspamd_log, uid_t uid, gid_t gid) } void -rspamd_log_close_priv (rspamd_logger_t *rspamd_log, uid_t uid, gid_t gid) +rspamd_log_close_priv (rspamd_logger_t *rspamd_log, gboolean termination, uid_t uid, gid_t gid) { gchar tmpbuf[256]; rspamd_log_flush (rspamd_log); @@ -349,12 +356,17 @@ rspamd_log_close_priv (rspamd_logger_t *rspamd_log, uid_t uid, gid_t gid) rspamd_log->enabled = FALSE; rspamd_log->opened = FALSE; } + + if (termination && rspamd_log->log_file) { + g_free (rspamd_log->log_file); + rspamd_log->log_file = NULL; + } } gint rspamd_log_reopen_priv (rspamd_logger_t *rspamd_log, uid_t uid, gid_t gid) { - rspamd_log_close_priv (rspamd_log, uid, gid); + rspamd_log_close_priv (rspamd_log, FALSE, uid, gid); if (rspamd_log_open_priv (rspamd_log, uid, gid) == 0) { msg_info ("log file reopened"); @@ -377,9 +389,9 @@ rspamd_log_open (rspamd_logger_t *logger) * Close log file or destroy other structures */ void -rspamd_log_close (rspamd_logger_t *logger) +rspamd_log_close (rspamd_logger_t *logger, gboolean termination) { - rspamd_log_close_priv (logger, -1, -1); + rspamd_log_close_priv (logger, termination, -1, -1); } /** @@ -445,7 +457,21 @@ rspamd_set_logger (struct rspamd_config *cfg, break; } - logger->cfg = cfg; + logger->log_type = cfg->log_type; + logger->log_facility = cfg->log_facility; + logger->log_level = cfg->log_level; + logger->log_buffered = cfg->log_buffered; + logger->log_silent_workers = cfg->log_silent_workers; + logger->log_buf_size = cfg->log_buf_size; + + if (logger->log_file) { + g_free (logger->log_file); + logger->log_file = NULL; + } + if (cfg->log_file) { + logger->log_file = g_strdup (cfg->log_file); + } + logger->flags = cfg->log_flags; /* Set up buffer */ @@ -534,7 +560,7 @@ rspamd_log_flush (rspamd_logger_t *rspamd_log) direct_write_log_line (rspamd_log, rspamd_log->io_buf.buf, rspamd_log->io_buf.used, - FALSE, rspamd_log->cfg->log_level); + FALSE, rspamd_log->log_level); rspamd_log->io_buf.used = 0; } } @@ -545,7 +571,7 @@ rspamd_logger_need_log (rspamd_logger_t *rspamd_log, GLogLevelFlags log_level, { g_assert (rspamd_log != NULL); - if ((log_level & RSPAMD_LOG_FORCED) || log_level <= rspamd_log->cfg->log_level) { + if ((log_level & RSPAMD_LOG_FORCED) || log_level <= rspamd_log->log_level) { return TRUE; } diff --git a/src/libutil/logger.h b/src/libutil/logger.h index 3b55246ea..bfb36ed1d 100644 --- a/src/libutil/logger.h +++ b/src/libutil/logger.h @@ -40,7 +40,7 @@ gint rspamd_log_open (rspamd_logger_t *logger); /** * Close log file or destroy other structures */ -void rspamd_log_close (rspamd_logger_t *logger); +void rspamd_log_close (rspamd_logger_t *logger, gboolean termination); /** * Close and open log again @@ -55,7 +55,7 @@ gint rspamd_log_open_priv (rspamd_logger_t *logger, uid_t uid, gid_t gid); /** * Close log file or destroy other structures for privileged processes */ -void rspamd_log_close_priv (rspamd_logger_t *logger, uid_t uid, gid_t gid); +void rspamd_log_close_priv (rspamd_logger_t *logger, gboolean termination, uid_t uid, gid_t gid); /** * Close and open log again for privileged processes diff --git a/src/log_helper.c b/src/log_helper.c index b62593db8..01494a447 100644 --- a/src/log_helper.c +++ b/src/log_helper.c @@ -227,8 +227,8 @@ start_log_helper (struct rspamd_worker *worker) close (ctx->pair[0]); rspamd_worker_block_signals (); - rspamd_log_close (worker->srv->logger); REF_RELEASE (ctx->cfg); + rspamd_log_close (worker->srv->logger, TRUE); exit (EXIT_SUCCESS); } diff --git a/src/lua_worker.c b/src/lua_worker.c index d5b651aaf..8aea54da8 100644 --- a/src/lua_worker.c +++ b/src/lua_worker.c @@ -412,8 +412,9 @@ start_lua_worker (struct rspamd_worker *worker) luaL_unref (L, LUA_REGISTRYINDEX, ctx->cbref_fin); } - rspamd_log_close (worker->srv->logger); REF_RELEASE (ctx->cfg); + rspamd_log_close (worker->srv->logger, TRUE); + exit (EXIT_SUCCESS); } diff --git a/src/rspamadm/rspamadm.c b/src/rspamadm/rspamadm.c index 9eeccb545..d5a656a51 100644 --- a/src/rspamadm/rspamadm.c +++ b/src/rspamadm/rspamadm.c @@ -498,8 +498,8 @@ main (gint argc, gchar **argv, gchar **env) cmd->run (0, NULL, cmd); } - rspamd_log_close (rspamd_main->logger); REF_RELEASE (rspamd_main->cfg); + rspamd_log_close (rspamd_main->logger, TRUE); g_free (rspamd_main); g_ptr_array_free (all_commands, TRUE); diff --git a/src/rspamd.c b/src/rspamd.c index 9339e0a3f..fdc9b1b49 100644 --- a/src/rspamd.c +++ b/src/rspamd.c @@ -294,6 +294,7 @@ reread_config (struct rspamd_main *rspamd_main) TRUE)) { rspamd_main->cfg = old_cfg; rspamd_log_close_priv (rspamd_main->logger, + FALSE, rspamd_main->workers_uid, rspamd_main->workers_gid); rspamd_set_logger (rspamd_main->cfg, g_quark_try_string ("main"), @@ -972,6 +973,7 @@ rspamd_hup_handler (gint signo, short what, gpointer arg) " is restarting"); g_hash_table_foreach (rspamd_main->workers, kill_old_workers, NULL); rspamd_log_close_priv (rspamd_main->logger, + FALSE, rspamd_main->workers_uid, rspamd_main->workers_gid); reread_config (rspamd_main); @@ -1288,8 +1290,8 @@ main (gint argc, gchar **argv, gchar **env) exit (EXIT_SUCCESS); } - rspamd_log_close_priv (rspamd_main->logger, rspamd_main->workers_uid, - rspamd_main->workers_gid); + rspamd_log_close_priv (rspamd_main->logger, FALSE, + rspamd_main->workers_uid, rspamd_main->workers_gid); if (config_test || dump_cache) { if (!load_rspamd_config (rspamd_main, rspamd_main->cfg, FALSE, 0, @@ -1516,8 +1518,8 @@ main (gint argc, gchar **argv, gchar **env) msg_info_main ("terminating..."); - rspamd_log_close (rspamd_main->logger); REF_RELEASE (rspamd_main->cfg); + rspamd_log_close (rspamd_main->logger, TRUE); g_hash_table_unref (rspamd_main->spairs); g_hash_table_unref (rspamd_main->workers); rspamd_mempool_delete (rspamd_main->server_pool); diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c index 276e304b9..88bc03bf0 100644 --- a/src/rspamd_proxy.c +++ b/src/rspamd_proxy.c @@ -2211,14 +2211,13 @@ start_rspamd_proxy (struct rspamd_worker *worker) { event_base_loop (ctx->ev_base, 0); rspamd_worker_block_signals (); - rspamd_log_close (worker->srv->logger); - if (ctx->has_self_scan) { rspamd_stat_close (); } rspamd_keypair_cache_destroy (ctx->keys_cache); REF_RELEASE (ctx->cfg); + rspamd_log_close (worker->srv->logger, TRUE); exit (EXIT_SUCCESS); } diff --git a/src/worker.c b/src/worker.c index f1b75b0d9..2ce029779 100644 --- a/src/worker.c +++ b/src/worker.c @@ -691,9 +691,9 @@ start_worker (struct rspamd_worker *worker) rspamd_worker_block_signals (); rspamd_stat_close (); - rspamd_log_close (worker->srv->logger); rspamd_keypair_cache_destroy (ctx->keys_cache); REF_RELEASE (ctx->cfg); + rspamd_log_close (worker->srv->logger, TRUE); exit (EXIT_SUCCESS); } -- 2.39.5