[Fix] Another try to fix race condition in the runtime destruction

diff --git a/src/libserver/rspamd_symcache.h b/src/libserver/rspamd_symcache.h
index 95ca614af8b2458349b07e1a75bbfa9642f69f56..8707c631fc073d9aeb25cde1f67eeab03ced3dc0 100644 (file)
--- a/src/libserver/rspamd_symcache.h
+++ b/src/libserver/rspamd_symcache.h
@@ -520,6 +520,12 @@ const struct rspamd_symcache_item_stat *
  * @param task
  */
 void rspamd_symcache_enable_profile (struct rspamd_task *task);
+
+/**
+ * Destroy internal state of the symcache runtime
+ * @param task
+ */
+void rspamd_symcache_runtime_destroy (struct rspamd_task *task);
 #ifdef  __cplusplus
 }
 #endif

diff --git a/src/libserver/symcache/symcache_c.cxx b/src/libserver/symcache/symcache_c.cxx
index 4d1f28800f7f612ff91a761af50d7d7382ffbe1c..580e0d137a6284121ee52aa17356a4a890fbf73c 100644 (file)
--- a/src/libserver/symcache/symcache_c.cxx
+++ b/src/libserver/symcache/symcache_c.cxx
@@ -668,4 +668,11 @@ rspamd_symcache_finalize_item(struct rspamd_task *task,
        auto *real_dyn_item = C_API_SYMCACHE_DYN_ITEM(item);
 
        cache_runtime->finalize_item(task, real_dyn_item);
+}
+
+void
+rspamd_symcache_runtime_destroy (struct rspamd_task *task)
+{
+       auto *cache_runtime = C_API_SYMCACHE_RUNTIME(task->symcache_runtime);
+       cache_runtime->savepoint_dtor();
 }
\ No newline at end of file

diff --git a/src/libserver/symcache/symcache_runtime.cxx b/src/libserver/symcache/symcache_runtime.cxx
index 4bfa99529c9c0b074ac811bb30203a6ab228607b..f406a1d5873a047748cc878d305bddcb9748f737 100644 (file)
--- a/src/libserver/symcache/symcache_runtime.cxx
+++ b/src/libserver/symcache/symcache_runtime.cxx
@@ -45,8 +45,6 @@ symcache_runtime::create(struct rspamd_task *task, symcache &cache) -> symcache_
                        sizeof(struct cache_dynamic_item) * cur_order->size());
 
        checkpoint->order = cache.get_cache_order();
-       rspamd_mempool_add_destructor(task->task_pool,
-                       symcache_runtime::savepoint_dtor, checkpoint);
 
        /* Calculate profile probability */
        ev_now_update_if_cheap(task->event_loop);
@@ -647,6 +645,7 @@ rspamd_symcache_delayed_item_fin(gpointer ud)
 {
        auto *cbd = (struct rspamd_symcache_delayed_cbdata *) ud;
 
+       cbd->event = nullptr;
        cbd->runtime->unset_slow();
        ev_timer_stop(cbd->task->event_loop, &cbd->tm);
 }
@@ -656,12 +655,15 @@ rspamd_symcache_delayed_item_cb(EV_P_ ev_timer *w, int what)
 {
        auto *cbd = (struct rspamd_symcache_delayed_cbdata *) w->data;
 
-       cbd->event = NULL;
+       if (cbd->event) {
+               cbd->event = nullptr;
 
-       /* Timer will be stopped here */
-       rspamd_session_remove_event (cbd->task->s,
-                       rspamd_symcache_delayed_item_fin, cbd);
-       cbd->runtime->process_item_rdeps(cbd->task, cbd->item);
+               /* Timer will be stopped here */
+               rspamd_session_remove_event (cbd->task->s,
+                               rspamd_symcache_delayed_item_fin, cbd);
+
+               cbd->runtime->process_item_rdeps(cbd->task, cbd->item);
+       }
 
 }
 
@@ -671,7 +673,7 @@ rspamd_delayed_timer_dtor(gpointer d)
        auto *cbd = (struct rspamd_symcache_delayed_cbdata *) d;
 
        if (cbd->event) {
-               /* Event has not been executed */
+               /* Event has not been executed, this will also stop a timer */
                rspamd_session_remove_event (cbd->task->s,
                                rspamd_symcache_delayed_item_fin, cbd);
                cbd->event = nullptr;
@@ -785,6 +787,11 @@ auto symcache_runtime::process_item_rdeps(struct rspamd_task *task, cache_item *
 {
        auto *cache_ptr = reinterpret_cast<symcache *>(task->cfg->cache);
 
+       // Avoid race condition with the runtime destruction and the delay timer
+       if (!order) {
+               return;
+       }
+
        for (const auto &rdep: item->rdeps) {
                if (rdep.item) {
                        auto *dyn_item = get_dynamic_item(rdep.item->id);

diff --git a/src/libserver/symcache/symcache_runtime.hxx b/src/libserver/symcache/symcache_runtime.hxx
index 237cad2d24a4069a9e273bdf66521d5219e7cebe..358cba4fbaaf33e68b69890da12038e1a99b61c1 100644 (file)
--- a/src/libserver/symcache/symcache_runtime.hxx
+++ b/src/libserver/symcache/symcache_runtime.hxx
@@ -60,13 +60,6 @@ class symcache_runtime {
        mutable struct cache_dynamic_item dynamic_items[];
        /* We allocate this structure merely in memory pool, so destructor is absent */
        ~symcache_runtime() = delete;
-       /* Dropper for a shared ownership */
-       static auto savepoint_dtor(void *ptr) -> void {
-               auto *real_savepoint = (symcache_runtime *)ptr;
-
-               /* Drop shared ownership */
-               real_savepoint->order.reset();
-       }
 
        auto process_symbol(struct rspamd_task *task, symcache &cache, cache_item *item,
                        cache_dynamic_item *dyn_item) -> bool;
@@ -78,6 +71,12 @@ class symcache_runtime {
                                                 cache_dynamic_item *dyn_item, bool check_only) -> bool;
 
 public:
+       /* Dropper for a shared ownership */
+       auto savepoint_dtor() -> void {
+
+               /* Drop shared ownership */
+               order.reset();
+       }
        /**
         * Creates a cache runtime using task mempool
         * @param task

diff --git a/src/libserver/task.c b/src/libserver/task.c
index 2b2dc727d5419ee5f432f18f9d6223a0e81b992d..46aabcb62cf3e6b9ab342423e9618674eac231e2 100644 (file)
--- a/src/libserver/task.c
+++ b/src/libserver/task.c
@@ -309,8 +309,17 @@ rspamd_task_free (struct rspamd_task *task)
                rspamd_message_unref (task->message);
 
                if (task->flags & RSPAMD_TASK_FLAG_OWN_POOL) {
+                       rspamd_mempool_destructors_enforce (task->task_pool);
+
+                       if (task->symcache_runtime) {
+                               rspamd_symcache_runtime_destroy (task);
+                       }
+
                        rspamd_mempool_delete (task->task_pool);
                }
+               else if (task->symcache_runtime) {
+                       rspamd_symcache_runtime_destroy (task);
+               }
        }
 }