Another fix to avoid race with settings - add reference counter.

diff --git a/src/settings.c b/src/settings.c
index 13e38fc90a850fc5a123444d7b72755355a85749..b5786d393a128b6ece351066ca5886845975abae 100644 (file)
--- a/src/settings.c
+++ b/src/settings.c
@@ -73,7 +73,40 @@ settings_free (gpointer data)
        if (s->blacklist) {
                g_hash_table_destroy (s->blacklist);
        }
-       g_free (s);
+       g_slice_free1 (sizeof (struct rspamd_settings), s);
+}
+
+static struct rspamd_settings *
+settings_ref (struct rspamd_settings *s)
+{
+       if (s == NULL) {
+               s = g_slice_alloc (sizeof (struct rspamd_settings));
+               s->metric_scores = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+               s->reject_scores = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+               s->metric_actions = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, settings_actions_free);
+               s->factors = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+               s->whitelist = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+               s->blacklist = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+               s->statfile_alias = NULL;
+               s->want_spam = FALSE;
+               s->ref_count = 1;
+       }
+       else {
+               s->ref_count ++;
+       }
+
+       return s;
+}
+
+static void
+settings_unref (struct rspamd_settings *s)
+{
+       if (s != NULL) {
+               s->ref_count --;
+               if (s->ref_count <= 0) {
+                       settings_free (s);
+               }
+       }
 }
 
 
@@ -127,7 +160,7 @@ json_fin_cb (memory_pool_t * pool, struct map_cb_data *data)
        struct metric_action           *new_act;
        struct rspamd_settings         *cur_settings;
        GList                          *cur_act;
-       gchar                           *cur_name;
+       gchar                          *cur_name;
        void                           *json_it;
        double                         *score;
 
@@ -172,26 +205,20 @@ json_fin_cb (memory_pool_t * pool, struct map_cb_data *data)
 
        nelts = json_array_size (js);
        for (i = 0; i < nelts; i++) {
-               cur_settings = g_malloc (sizeof (struct rspamd_settings));
-               cur_settings->metric_scores = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-               cur_settings->reject_scores = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-               cur_settings->metric_actions = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, settings_actions_free);
-               cur_settings->factors = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-               cur_settings->whitelist = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-               cur_settings->blacklist = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-               cur_settings->statfile_alias = NULL;
-               cur_settings->want_spam = FALSE;
+               cur_settings = settings_ref (NULL);
 
                cur_elt = json_array_get (js, i);
                if (!cur_elt || !json_is_object (cur_elt)) {
                        json_decref (js);
                        msg_err ("loaded json is not an object");
+                       settings_unref (cur_settings);
                        return;
                }
                cur_nm = json_object_get (cur_elt, "name");
                if (cur_nm == NULL || !json_is_string (cur_nm)) {
                        json_decref (js);
                        msg_err ("name is not a string or not exists");
+                       settings_unref (cur_settings);
                        return;
                }
                cur_name = g_strdup (json_string_value (cur_nm));
@@ -315,7 +342,7 @@ json_fin_cb (memory_pool_t * pool, struct map_cb_data *data)
                                cur_settings->want_spam = TRUE;
                        }
                }
-               g_hash_table_insert (((struct json_buf *)data->cur_data)->table, cur_name, cur_settings);
+               g_hash_table_replace (((struct json_buf *)data->cur_data)->table, cur_name, cur_settings);
        }
        json_decref (js);
 }
@@ -342,8 +369,10 @@ read_settings (const gchar *path, struct config_file *cfg, GHashTable * table)
 void
 init_settings (struct config_file *cfg)
 {
-       cfg->domain_settings = g_hash_table_new_full (rspamd_strcase_hash, rspamd_strcase_equal, g_free, settings_free);
-       cfg->user_settings = g_hash_table_new_full (rspamd_strcase_hash, rspamd_strcase_equal, g_free, settings_free);
+       cfg->domain_settings = g_hash_table_new_full (rspamd_strcase_hash, rspamd_strcase_equal,
+                       g_free, (GDestroyNotify)settings_unref);
+       cfg->user_settings = g_hash_table_new_full (rspamd_strcase_hash, rspamd_strcase_equal,
+                       g_free, (GDestroyNotify)settings_unref);
 }
 
 static                          gboolean
@@ -547,17 +576,15 @@ apply_metric_settings (struct worker_task *task, struct metric *metric, struct m
        if (check_setting (task, &us, &ds)) {
                if (us != NULL || ds != NULL) {
                        if (us != NULL) {
-                               g_hash_table_ref (task->cfg->user_settings);
-                               res->user_settings = us;
-                               memory_pool_add_destructor (task->task_pool, (pool_destruct_func)g_hash_table_unref,
-                                               task->cfg->user_settings);
+                               res->user_settings = settings_ref (us);
+                               memory_pool_add_destructor (task->task_pool, (pool_destruct_func)settings_unref,
+                                               us);
                        }
                        if (ds != NULL) {
                                /* Need to ref hash table to avoid occasional data corruption */
-                               g_hash_table_ref (task->cfg->domain_settings);
-                               res->domain_settings = ds;
-                               memory_pool_add_destructor (task->task_pool, (pool_destruct_func)g_hash_table_unref,
-                                               task->cfg->domain_settings);
+                               res->domain_settings = settings_ref (ds);
+                               memory_pool_add_destructor (task->task_pool, (pool_destruct_func)settings_unref,
+                                               ds);
                        }
                }
                else {

diff --git a/src/settings.h b/src/settings.h
index 5d77d429bd6aa0700d32f1b503dd52c1b7c7c4b0..fb8f8681ea2d9603eecea3a20f5101ff862ad5b6 100644 (file)
--- a/src/settings.h
+++ b/src/settings.h
@@ -13,6 +13,7 @@ struct rspamd_settings {
        GHashTable *blacklist;                          /**< hash table of whitelist for this setting                   */
        gchar *statfile_alias;                          /**< alias for statfile used                                                    */
        gboolean want_spam;                                     /**< if true disable rspamd checks                                              */
+       gint ref_count;                                         /**< reference counter                                                                  */
 };