From: Vsevolod Stakhov Date: Tue, 28 Oct 2025 14:23:10 +0000 (+0000) Subject: [Rework] Add CFG_REF_* macros with debug logging for config refcounting X-Git-Tag: 3.14.0~32^2~7 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a874072aa9a50739f49b49c343b49e18f8d74a8e;p=thirdparty%2Frspamd.git [Rework] Add CFG_REF_* macros with debug logging for config refcounting Introduce specialized refcount macros for rspamd_config that provide debug-level logging for better debugging of configuration lifecycle: - CFG_REF_INIT_RETAIN: Initialize refcount to 1 - CFG_REF_RETAIN: Increment refcount - CFG_REF_RELEASE: Decrement refcount and destroy if reaches 0 These macros use G_LOG_LEVEL_DEBUG to avoid cluttering info logs with refcounting details, while still providing visibility when debugging config lifetime issues. Also update all workers and components to use the new CFG_REF_* macros instead of generic REF_* macros for better tracking. --- diff --git a/src/controller.c b/src/controller.c index 98ff12453d..c575fbbbc1 100644 --- a/src/controller.c +++ b/src/controller.c @@ -3602,7 +3602,7 @@ rspamd_controller_finish_handler(struct rspamd_http_connection_entry *conn_ent) session->wrk->nconns--; rspamd_inet_address_free(session->from_addr); - REF_RELEASE(session->cfg); + CFG_REF_RELEASE(session->cfg); if (session->pool) { msg_debug_session("destroy session %p", session); @@ -3640,7 +3640,7 @@ rspamd_controller_accept_socket(EV_P_ ev_io *w, int revents) session->ctx = ctx; session->cfg = ctx->cfg; session->lang_det = ctx->lang_det; - REF_RETAIN(session->cfg); + CFG_REF_RETAIN(session->cfg); session->from_addr = addr; session->wrk = worker; @@ -4072,6 +4072,7 @@ start_controller_worker(struct rspamd_worker *worker) ctx->worker = worker; ctx->cfg = worker->srv->cfg; + CFG_REF_RETAIN(ctx->cfg); ctx->srv = worker->srv; ctx->custom_commands = g_hash_table_new(rspamd_strcase_hash, rspamd_strcase_equal); @@ -4280,7 +4281,8 @@ start_controller_worker(struct rspamd_worker *worker) g_hash_table_unref(ctx->plugins); g_hash_table_unref(ctx->custom_commands); - REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); rspamd_log_close(worker->srv->logger); rspamd_unset_crash_handler(worker->srv); diff --git a/src/fuzzy_storage.c b/src/fuzzy_storage.c index 67f2a1edbc..05bd11fc3e 100644 --- a/src/fuzzy_storage.c +++ b/src/fuzzy_storage.c @@ -4172,6 +4172,7 @@ start_fuzzy(struct rspamd_worker *worker) ctx->peer_fd = -1; ctx->worker = worker; ctx->cfg = worker->srv->cfg; + CFG_REF_RETAIN(ctx->cfg); ctx->resolver = rspamd_dns_resolver_init(worker->srv->logger, ctx->event_loop, worker->srv->cfg); @@ -4460,7 +4461,8 @@ start_fuzzy(struct rspamd_worker *worker) kh_destroy(fuzzy_key_ids_set, ctx->weak_ids); } - REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); rspamd_log_close(worker->srv->logger); rspamd_unset_crash_handler(worker->srv); diff --git a/src/hs_helper.c b/src/hs_helper.c index f3edbd64bb..27e348f380 100644 --- a/src/hs_helper.c +++ b/src/hs_helper.c @@ -624,6 +624,7 @@ start_hs_helper(struct rspamd_worker *worker) g_assert(rspamd_worker_check_context(worker->ctx, rspamd_hs_helper_magic)); ctx->cfg = worker->srv->cfg; + CFG_REF_RETAIN(ctx->cfg); if (ctx->hs_dir == NULL) { ctx->hs_dir = ctx->cfg->hs_cache_dir; @@ -655,7 +656,8 @@ start_hs_helper(struct rspamd_worker *worker) rspamd_worker_block_signals(); rspamd_log_close(worker->srv->logger); - REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); rspamd_unset_crash_handler(worker->srv); exit(EXIT_SUCCESS); diff --git a/src/libcryptobox/cryptobox.c b/src/libcryptobox/cryptobox.c index 190d0e4a3f..ab7d35c7a9 100644 --- a/src/libcryptobox/cryptobox.c +++ b/src/libcryptobox/cryptobox.c @@ -363,9 +363,10 @@ rspamd_cryptobox_init(void) void rspamd_cryptobox_deinit(struct rspamd_cryptobox_library_ctx *ctx) { - if (ctx) { + if (ctx && cryptobox_loaded) { g_free(ctx->cpu_extensions); g_free(ctx); + cryptobox_loaded = FALSE; } } diff --git a/src/libserver/cfg_file.h b/src/libserver/cfg_file.h index c16db6fe98..aa3172bf13 100644 --- a/src/libserver/cfg_file.h +++ b/src/libserver/cfg_file.h @@ -914,6 +914,49 @@ extern unsigned int rspamd_config_log_id; RSPAMD_LOG_FUNC, \ __VA_ARGS__) +/** + * Special refcount macros for rspamd_config with logging + */ +#define CFG_REF_INIT_RETAIN(cfg, dtor_cb) \ + do { \ + if ((cfg) != NULL) { \ + (cfg)->ref.refcount = 1; \ + (cfg)->ref.dtor = (ref_dtor_cb_t) (dtor_cb); \ + rspamd_default_log_function(G_LOG_LEVEL_DEBUG, "config", NULL, \ + G_STRFUNC, "CFG_REF_INIT_RETAIN %p at %s:%d refcount=1", \ + (void *) (cfg), __FILE__, __LINE__); \ + } \ + } while (0) + +#define CFG_REF_RETAIN(cfg) \ + do { \ + if ((cfg) != NULL) { \ + (cfg)->ref.refcount++; \ + rspamd_default_log_function(G_LOG_LEVEL_DEBUG, "config", NULL, \ + G_STRFUNC, "CFG_REF_RETAIN %p at %s:%d refcount=%ud", \ + (void *) (cfg), __FILE__, __LINE__, (cfg)->ref.refcount); \ + } \ + } while (0) + +#define CFG_REF_RELEASE(cfg) \ + do { \ + if ((cfg) != NULL) { \ + unsigned int _old_rc = (cfg)->ref.refcount; \ + if (--(cfg)->ref.refcount == 0 && (cfg)->ref.dtor) { \ + rspamd_default_log_function(G_LOG_LEVEL_DEBUG, "config", NULL, \ + G_STRFUNC, "CFG_REF_RELEASE %p at %s:%d refcount=%ud->0 DESTROYING", \ + (void *) (cfg), __FILE__, __LINE__, _old_rc); \ + (cfg)->ref.dtor(cfg); \ + } \ + else { \ + rspamd_default_log_function(G_LOG_LEVEL_DEBUG, "config", NULL, \ + G_STRFUNC, "CFG_REF_RELEASE %p at %s:%d refcount=%ud->%ud", \ + (void *) (cfg), __FILE__, __LINE__, _old_rc, \ + (cfg)->ref.refcount); \ + } \ + } \ + } while (0) + #ifdef __cplusplus } #endif diff --git a/src/libserver/cfg_rcl.cxx b/src/libserver/cfg_rcl.cxx index 91503ccb4f..3c13487400 100644 --- a/src/libserver/cfg_rcl.cxx +++ b/src/libserver/cfg_rcl.cxx @@ -3720,7 +3720,7 @@ void rspamd_rcl_maybe_apply_lua_transform(struct rspamd_config *cfg) msg_info_config("configuration has been transformed in Lua"); } - /* error function */ + /* Clear stack completely */ lua_settop(L, 0); } @@ -3744,6 +3744,12 @@ rspamd_rcl_decrypt_handler(struct ucl_parser *parser, return true; } +static void +rspamd_rcl_jinja_free(unsigned char *data, size_t len, void *user_data) +{ + UCL_FREE(len, data); +} + static bool rspamd_rcl_jinja_handler(struct ucl_parser *parser, const unsigned char *source, size_t source_len, @@ -3905,6 +3911,7 @@ rspamd_config_parse_ucl(struct rspamd_config *cfg, jinja_handler->user_data = cfg; jinja_handler->flags = UCL_SPECIAL_HANDLER_PREPROCESS_ALL; jinja_handler->handler = rspamd_rcl_jinja_handler; + jinja_handler->free_function = rspamd_rcl_jinja_free; ucl_parser_add_special_handler(parser.get(), jinja_handler); } @@ -3916,6 +3923,14 @@ rspamd_config_parse_ucl(struct rspamd_config *cfg, return FALSE; } + /* Free old UCL object if it exists (e.g., after lua_cfg_transform) */ + if (cfg->cfg_ucl_obj) { + ucl_object_unref(cfg->cfg_ucl_obj); + } + if (cfg->config_comments) { + ucl_object_unref(cfg->config_comments); + } + cfg->cfg_ucl_obj = ucl_parser_get_object(parser.get()); cfg->config_comments = ucl_object_ref(ucl_parser_get_comments(parser.get())); diff --git a/src/libserver/rspamd_control.c b/src/libserver/rspamd_control.c index 2e1b5a110b..8b43ddec70 100644 --- a/src/libserver/rspamd_control.c +++ b/src/libserver/rspamd_control.c @@ -742,7 +742,7 @@ rspamd_control_default_cmd_handler(int fd, } rep.reply.reresolve.status = 0; - REF_RELEASE(cfg); + CFG_REF_RELEASE(cfg); } else { rep.reply.reresolve.status = EINVAL; diff --git a/src/libserver/task.c b/src/libserver/task.c index 7e90d661b5..da37e4af06 100644 --- a/src/libserver/task.c +++ b/src/libserver/task.c @@ -91,7 +91,7 @@ rspamd_task_new(struct rspamd_worker *worker, if (cfg) { new_task->cfg = cfg; - REF_RETAIN(cfg); + CFG_REF_RETAIN(cfg); if (cfg->check_all_filters) { new_task->flags |= RSPAMD_TASK_FLAG_PASS_ALL; @@ -289,7 +289,7 @@ void rspamd_task_free(struct rspamd_task *task) (double) task->cfg->full_gc_iters / 2); } - REF_RELEASE(task->cfg); + CFG_REF_RELEASE(task->cfg); } kh_destroy(rspamd_req_headers_hash, task->request_headers); diff --git a/src/rspamadm/configtest.c b/src/rspamadm/configtest.c index 67f2e0bb5c..dae90eabd9 100644 --- a/src/rspamadm/configtest.c +++ b/src/rspamadm/configtest.c @@ -84,11 +84,13 @@ rspamadm_configtest(int argc, char **argv, const struct rspamadm_command *cmd) GOptionContext *context; GError *error = NULL; const char *confdir; - struct rspamd_config *cfg = rspamd_main->cfg; + struct rspamd_config *cfg; gboolean ret = TRUE; worker_t **pworker; const uint64_t *log_cnt; + cfg = rspamd_config_new(RSPAMD_CONFIG_INIT_DEFAULT | RSPAMD_CONFIG_INIT_WIPE_LUA_MEM); + cfg->libs_ctx = rspamd_init_libs(); context = g_option_context_new( "configtest - perform configuration file test"); g_option_context_set_summary(context, @@ -137,7 +139,7 @@ rspamadm_configtest(int argc, char **argv, const struct rspamadm_command *cmd) /* Do post-load actions */ rspamd_lua_post_load_config(cfg); - if (!rspamd_init_filters(rspamd_main->cfg, false, strict)) { + if (!rspamd_init_filters(cfg, false, strict)) { ret = FALSE; } @@ -184,6 +186,7 @@ rspamadm_configtest(int argc, char **argv, const struct rspamadm_command *cmd) rspamd_printf("syntax %s\n", ret ? "OK" : "BAD"); } + CFG_REF_RELEASE(cfg); if (!ret) { exit(EXIT_FAILURE); } diff --git a/src/rspamadm/rspamadm.c b/src/rspamadm/rspamadm.c index 4a19aa667e..655b8606d0 100644 --- a/src/rspamadm/rspamadm.c +++ b/src/rspamadm/rspamadm.c @@ -607,7 +607,7 @@ end: rspamd_session_destroy(rspamadm_session); g_option_context_free(context); rspamd_dns_resolver_deinit(resolver); - REF_RELEASE(rspamd_main->cfg); + CFG_REF_RELEASE(rspamd_main->cfg); rspamd_http_context_free(rspamd_main->http_ctx); rspamd_log_close(rspamd_main->logger); rspamd_url_deinit(); diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c index aed9076e82..683f35b079 100644 --- a/src/rspamd_proxy.c +++ b/src/rspamd_proxy.c @@ -3092,6 +3092,7 @@ start_rspamd_proxy(struct rspamd_worker *worker) g_assert(rspamd_worker_check_context(worker->ctx, rspamd_rspamd_proxy_magic)); ctx->cfg = worker->srv->cfg; + CFG_REF_RETAIN(ctx->cfg); ctx->srv = worker->srv; ctx->event_loop = rspamd_prepare_worker(worker, "rspamd_proxy", proxy_accept_socket); @@ -3166,7 +3167,8 @@ start_rspamd_proxy(struct rspamd_worker *worker) rspamd_controller_on_terminate(worker, NULL); } - REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); rspamd_log_close(worker->srv->logger); rspamd_unset_crash_handler(worker->srv); diff --git a/src/worker.c b/src/worker.c index 5f8ba76c54..5fdb2789f4 100644 --- a/src/worker.c +++ b/src/worker.c @@ -486,6 +486,7 @@ start_worker(struct rspamd_worker *worker) g_assert(rspamd_worker_check_context(worker->ctx, rspamd_worker_magic)); ctx->cfg = worker->srv->cfg; + CFG_REF_RETAIN(ctx->cfg); ctx->event_loop = rspamd_prepare_worker(worker, "normal", accept_socket); rspamd_symcache_start_refresh(worker->srv->cfg->cache, ctx->event_loop, worker); @@ -527,7 +528,8 @@ start_worker(struct rspamd_worker *worker) } rspamd_stat_close(); - REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); + CFG_REF_RELEASE(ctx->cfg); rspamd_log_close(worker->srv->logger); rspamd_unset_crash_handler(worker->srv); diff --git a/test/rspamd_upstream_test.c b/test/rspamd_upstream_test.c index 998a51f26f..f6cc636f69 100644 --- a/test/rspamd_upstream_test.c +++ b/test/rspamd_upstream_test.c @@ -178,5 +178,5 @@ void rspamd_upstream_test_func(void) g_assert(rspamd_upstreams_alive(ls) == 3); rspamd_upstreams_destroy(ls); - REF_RELEASE(cfg); + CFG_REF_RELEASE(cfg); }