[CritFix] Fix maps race conditions on reload

10 files changed, 73 insertions, 71 deletions

diff --git a/src/controller.c b/src/controller.c
index 4c08b5e13..27e1bd6e7 100644
--- a/src/controller.c
+++ b/src/controller.c
@@ -3802,10 +3802,12 @@ start_controller_worker (struct rspamd_worker *worker)
 			rspamd_worker_init_monitored (worker, ctx->ev_base, ctx->resolver);
 		}
 
-		rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, TRUE);
+		rspamd_map_watch (worker->srv->cfg, ctx->ev_base,
+				ctx->resolver, worker, TRUE);
 	}
 	else {
-		rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, FALSE);
+		rspamd_map_watch (worker->srv->cfg, ctx->ev_base,
+				ctx->resolver, worker, FALSE);
 	}
 
 	rspamd_lua_run_postloads (ctx->cfg->lua_state, ctx->cfg, ctx->ev_base, worker);
diff --git a/src/fuzzy_storage.c b/src/fuzzy_storage.c
index 234822018..5499fa732 100644
--- a/src/fuzzy_storage.c
+++ b/src/fuzzy_storage.c
@@ -2903,7 +2903,7 @@ start_fuzzy (struct rspamd_worker *worker)
 	ctx->resolver = dns_resolver_init (worker->srv->logger,
 				ctx->ev_base,
 				worker->srv->cfg);
-	rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, 0);
+	rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, worker, 0);
 
 	/* Get peer pipe */
 	memset (&srv_cmd, 0, sizeof (srv_cmd));
diff --git a/src/libserver/worker_util.c b/src/libserver/worker_util.c
index e706e95f4..774805544 100644
--- a/src/libserver/worker_util.c
+++ b/src/libserver/worker_util.c
@@ -386,9 +386,6 @@ rspamd_worker_stop_accept (struct rspamd_worker *worker)
 
 	g_hash_table_unref (worker->signal_events);
 #endif
-
-	/* Cleanup maps */
-	rspamd_map_remove_all (worker->srv->cfg);
 }
 
 static rspamd_fstring_t *
diff --git a/src/libutil/map.c b/src/libutil/map.c
index 12e4f0a4e..203af03c7 100644
--- a/src/libutil/map.c
+++ b/src/libutil/map.c
@@ -1058,7 +1058,7 @@ rspamd_map_schedule_periodic (struct rspamd_map *map,
 	gdouble timeout;
 	struct map_periodic_cbdata *cbd;
 
-	if (map->scheduled_check) {
+	if (map->scheduled_check || (map->wrk && map->wrk->wanna_die)) {
 		/* Do not schedule check if some check is already scheduled */
 		return;
 	}
@@ -1583,45 +1583,49 @@ rspamd_map_periodic_callback (gint fd, short what, void *ud)
 		return;
 	}
 
-	bk = g_ptr_array_index (cbd->map->backends, cbd->cur_backend);
-	g_assert (bk != NULL);
-
-	if (cbd->need_modify) {
-		/* Load data from the next backend */
-		switch (bk->protocol) {
-		case MAP_PROTO_HTTP:
-		case MAP_PROTO_HTTPS:
-			rspamd_map_http_read_callback (fd, what, cbd);
-			break;
-		case MAP_PROTO_FILE:
-			rspamd_map_file_read_callback (fd, what, cbd);
-			break;
-		case MAP_PROTO_STATIC:
-			rspamd_map_static_read_callback (fd, what, cbd);
-			break;
-		}
-	}
-	else {
-		/* Check the next backend */
-		switch (bk->protocol) {
-		case MAP_PROTO_HTTP:
-		case MAP_PROTO_HTTPS:
-			rspamd_map_http_check_callback (fd, what, cbd);
-			break;
-		case MAP_PROTO_FILE:
-			rspamd_map_file_check_callback (fd, what, cbd);
-			break;
-		case MAP_PROTO_STATIC:
-			rspamd_map_static_check_callback (fd, what, cbd);
-			break;
+	if (!(cbd->map->wrk && cbd->map->wrk->wanna_die)) {
+		bk = g_ptr_array_index (cbd->map->backends, cbd->cur_backend);
+		g_assert (bk != NULL);
+
+		if (cbd->need_modify) {
+			/* Load data from the next backend */
+			switch (bk->protocol) {
+			case MAP_PROTO_HTTP:
+			case MAP_PROTO_HTTPS:
+				rspamd_map_http_read_callback (fd, what, cbd);
+				break;
+			case MAP_PROTO_FILE:
+				rspamd_map_file_read_callback (fd, what, cbd);
+				break;
+			case MAP_PROTO_STATIC:
+				rspamd_map_static_read_callback (fd, what, cbd);
+				break;
+			}
+		} else {
+			/* Check the next backend */
+			switch (bk->protocol) {
+			case MAP_PROTO_HTTP:
+			case MAP_PROTO_HTTPS:
+				rspamd_map_http_check_callback (fd, what, cbd);
+				break;
+			case MAP_PROTO_FILE:
+				rspamd_map_file_check_callback (fd, what, cbd);
+				break;
+			case MAP_PROTO_STATIC:
+				rspamd_map_static_check_callback (fd, what, cbd);
+				break;
+			}
 		}
 	}
 }
 
 /* Start watching event for all maps */
 void
-rspamd_map_watch (struct rspamd_config *cfg, struct event_base *ev_base,
-		struct rspamd_dns_resolver *resolver, gboolean active_http)
+rspamd_map_watch (struct rspamd_config *cfg,
+				  struct event_base *ev_base,
+				  struct rspamd_dns_resolver *resolver,
+				  struct rspamd_worker *worker,
+				  gboolean active_http)
 {
 	GList *cur = cfg->maps;
 	struct rspamd_map *map;
@@ -1631,6 +1635,7 @@ rspamd_map_watch (struct rspamd_config *cfg, struct event_base *ev_base,
 		map = cur->data;
 		map->ev_base = ev_base;
 		map->r = resolver;
+		map->wrk = worker;
 
 		if (active_http) {
 			map->active_http = active_http;
@@ -1905,9 +1910,9 @@ rspamd_map_parse_backend (struct rspamd_config *cfg, const gchar *map_line)
 		if (access (bk->uri, R_OK) == -1) {
 			if (errno != ENOENT) {
 				msg_err_config ("cannot open file '%s': %s", bk->uri, strerror (errno));
-				return NULL;
-
+				goto err;
 			}
+
 			msg_info_config (
 					"map '%s' is not found, but it can be loaded automatically later",
 					bk->uri);
@@ -1928,7 +1933,7 @@ rspamd_map_parse_backend (struct rspamd_config *cfg, const gchar *map_line)
 		else {
 			if (!(up.field_set & 1 << UF_HOST)) {
 				msg_err_config ("cannot parse HTTP url: %s: no host", bk->uri);
-				return NULL;
+				goto err;
 			}
 
 			tok.begin = bk->uri + up.field_data[UF_HOST].off;
@@ -1956,7 +1961,8 @@ rspamd_map_parse_backend (struct rspamd_config *cfg, const gchar *map_line)
 		}
 
 		bk->data.hd = hdata;
-	}else if (bk->protocol == MAP_PROTO_STATIC) {
+	}
+	else if (bk->protocol == MAP_PROTO_STATIC) {
 		sdata = g_malloc0 (sizeof (*sdata));
 		bk->data.sd = sdata;
 	}
diff --git a/src/libutil/map.h b/src/libutil/map.h
index e12df4369..9c7485c58 100644
--- a/src/libutil/map.h
+++ b/src/libutil/map.h
@@ -78,8 +78,11 @@ struct rspamd_map* rspamd_map_add_from_ucl (struct rspamd_config *cfg,
 /**
  * Start watching of maps by adding events to libevent event loop
  */
-void rspamd_map_watch (struct rspamd_config *cfg, struct event_base *ev_base,
-		struct rspamd_dns_resolver *resolver, gboolean active_http);
+void rspamd_map_watch (struct rspamd_config *cfg,
+					   struct event_base *ev_base,
+					   struct rspamd_dns_resolver *resolver,
+					   struct rspamd_worker *worker,
+					   gboolean active_http);
 
 /**
  * Remove all maps watched (remove events)
diff --git a/src/libutil/map_private.h b/src/libutil/map_private.h
index b8543356b..1b40e8e8a 100644
--- a/src/libutil/map_private.h
+++ b/src/libutil/map_private.h
@@ -120,6 +120,7 @@ struct rspamd_map {
 	map_dtor_t dtor;
 	void **user_data;
 	struct event_base *ev_base;
+	struct rspamd_worker *wrk;
 	gchar *description;
 	gchar *name;
 	guint32 id;
diff --git a/src/plugins/surbl.c b/src/plugins/surbl.c
index 238f14fb2..64b1b14f6 100644
--- a/src/plugins/surbl.c
+++ b/src/plugins/surbl.c
@@ -230,14 +230,18 @@ read_exceptions_list (gchar * chunk,
 	if (data->cur_data == NULL) {
 		t = data->prev_data;
 
-		for (i = 0; i < MAX_LEVELS; i++) {
-			if (t[i] != NULL) {
-				g_hash_table_destroy (t[i]);
+		if (t) {
+			for (i = 0; i < MAX_LEVELS; i++) {
+				if (t[i] != NULL) {
+					g_hash_table_destroy (t[i]);
+				}
+				t[i] = NULL;
 			}
-			t[i] = NULL;
+
+			g_free (t);
 		}
 
-		data->cur_data = data->prev_data;
+		data->cur_data = g_malloc0 (MAX_LEVELS * sizeof (GHashTable *));
 	}
 
 	return rspamd_parse_kv_list (
@@ -283,6 +287,8 @@ dtor_exceptions_list (struct map_cb_data *data)
 			}
 			t[i] = NULL;
 		}
+
+		g_free (t);
 	}
 }
 
@@ -397,15 +403,12 @@ surbl_module_init (struct rspamd_config *cfg, struct module_ctx **ctx)
 
 	surbl_module_ctx->use_redirector = 0;
 	surbl_module_ctx->suffixes = NULL;
-	surbl_module_ctx->surbl_pool = rspamd_mempool_new (rspamd_mempool_suggest_size (), NULL);
+	surbl_module_ctx->surbl_pool = rspamd_mempool_new (rspamd_mempool_suggest_size (),
+			NULL);
 
 	surbl_module_ctx->redirectors = NULL;
 	surbl_module_ctx->whitelist = NULL;
-	rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool,
-			(rspamd_mempool_destruct_t) rspamd_map_helper_destroy_hash,
-			surbl_module_ctx->whitelist);
-	surbl_module_ctx->exceptions = rspamd_mempool_alloc0 (
-			surbl_module_ctx->surbl_pool, MAX_LEVELS * sizeof (GHashTable *));
+	surbl_module_ctx->exceptions = NULL;
 	surbl_module_ctx->redirector_cbid = -1;
 
 
@@ -1081,16 +1084,7 @@ surbl_module_reconfig (struct rspamd_config *cfg)
 	surbl_module_ctx->redirectors = NULL;
 	surbl_module_ctx->whitelist = NULL;
 	/* Zero exceptions hashes */
-	surbl_module_ctx->exceptions = rspamd_mempool_alloc0 (
-		surbl_module_ctx->surbl_pool,
-		MAX_LEVELS * sizeof (GHashTable *));
-	/* Register destructors */
-	rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool,
-		(rspamd_mempool_destruct_t) rspamd_map_helper_destroy_hash,
-		surbl_module_ctx->whitelist);
-	rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool,
-		(rspamd_mempool_destruct_t) g_hash_table_destroy,
-		surbl_module_ctx->redirector_tlds);
+	surbl_module_ctx->exceptions = NULL;
 
 	rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool,
 		(rspamd_mempool_destruct_t) g_list_free,
@@ -1178,7 +1172,7 @@ format_surbl_request (rspamd_mempool_t * pool,
 		/* Not a numeric url */
 		result = rspamd_mempool_alloc (pool, len);
 		/* Now we should try to check for exceptions */
-		if (!forced) {
+		if (!forced && surbl_module_ctx->exceptions) {
 			for (i = MAX_LEVELS - 1; i >= 0; i--) {
 				t = surbl_module_ctx->exceptions[i];
 				if (t != NULL && dots_num >= i + 1) {
diff --git a/src/rspamd.c b/src/rspamd.c
index b5454f38d..cba581419 100644
--- a/src/rspamd.c
+++ b/src/rspamd.c
@@ -970,7 +970,6 @@ rspamd_hup_handler (gint signo, short what, gpointer arg)
 			RVERSION
 			" is restarting");
 	g_hash_table_foreach (rspamd_main->workers, kill_old_workers, NULL);
-	rspamd_map_remove_all (rspamd_main->cfg);
 	rspamd_log_close_priv (rspamd_main->logger,
 				rspamd_main->workers_uid,
 				rspamd_main->workers_gid);
diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c
index d94ab6455..055421a41 100644
--- a/src/rspamd_proxy.c
+++ b/src/rspamd_proxy.c
@@ -2165,7 +2165,7 @@ start_rspamd_proxy (struct rspamd_worker *worker) {
 			ctx->ev_base,
 			worker->srv->cfg);
 	double_to_tv (ctx->timeout, &ctx->io_tv);
-	rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, 0);
+	rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, worker, 0);
 
 	rspamd_upstreams_library_config (worker->srv->cfg, ctx->cfg->ups_ctx,
 			ctx->ev_base, ctx->resolver->r);
diff --git a/src/worker.c b/src/worker.c
index 6b02b5753..f26a86ff7 100644
--- a/src/worker.c
+++ b/src/worker.c
@@ -676,7 +676,7 @@ start_worker (struct rspamd_worker *worker)
 	ctx->resolver = dns_resolver_init (worker->srv->logger,
 			ctx->ev_base,
 			worker->srv->cfg);
-	rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, 0);
+	rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, worker, 0);
 	rspamd_upstreams_library_config (worker->srv->cfg, ctx->cfg->ups_ctx,
 			ctx->ev_base, ctx->resolver->r);