commit 5c0c81f: [Fix] Fix various symcache issues

[Vsevolod Stakhov]

Author: Vsevolod Stakhov
Date: 2022-05-01 13:58:18 +0100
URL: https://github.com/rspamd/rspamd/commit/5c0c81f08cc75d6f8bde748265a6eb86ba904f3c (HEAD -> master)

[Fix] Fix various symcache issues

---
 src/libserver/symcache/symcache_c.cxx       |  5 +++-
 src/libserver/symcache/symcache_impl.cxx    | 14 +++++------
 src/libserver/symcache/symcache_item.cxx    | 36 +++++++++++++++++++++++++----
 src/libserver/symcache/symcache_item.hxx    |  2 ++
 src/libserver/symcache/symcache_runtime.cxx |  1 +
 5 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/src/libserver/symcache/symcache_c.cxx b/src/libserver/symcache/symcache_c.cxx
index 7af45ac64..d4ebf4be3 100644
--- a/src/libserver/symcache/symcache_c.cxx
+++ b/src/libserver/symcache/symcache_c.cxx
@@ -181,7 +181,10 @@ void
 rspamd_symcache_inc_frequency(struct rspamd_symcache *_cache, struct rspamd_symcache_item *item)
 {
 	auto *real_item = C_API_SYMCACHE_ITEM(item);
-	real_item->inc_frequency();
+
+	if (real_item) {
+		real_item->inc_frequency();
+	}
 }
 
 void
diff --git a/src/libserver/symcache/symcache_impl.cxx b/src/libserver/symcache/symcache_impl.cxx
index b8e2ff2f9..0878ae32b 100644
--- a/src/libserver/symcache/symcache_impl.cxx
+++ b/src/libserver/symcache/symcache_impl.cxx
@@ -88,10 +88,6 @@ auto symcache::init() -> bool
 		it->process_deps(*this);
 	}
 
-	for (auto &it: virtual_symbols) {
-		it->process_deps(*this);
-	}
-
 	/* Sorting stuff */
 	auto postfilters_cmp = [](const auto &it1, const auto &it2) -> int {
 		if (it1->priority > it2->priority) {
@@ -404,9 +400,9 @@ auto symcache::add_dependency(int id_from, std::string_view to, int virtual_id_f
 
 
 	if (virtual_id_from >= 0) {
-		g_assert (virtual_id_from < (gint) virtual_symbols.size());
+		g_assert (virtual_id_from < (gint) items_by_id.size());
 		/* We need that for settings id propagation */
-		const auto &vsource = virtual_symbols[virtual_id_from];
+		const auto &vsource = items_by_id[virtual_id_from];
 		g_assert (vsource.get() != nullptr);
 		vsource->deps.emplace_back(cache_item_ptr{nullptr},
 				std::string(to),
@@ -754,9 +750,13 @@ auto symcache::validate(bool strict) -> bool
 
 		if (item->is_virtual()) {
 			if (!(item->flags & SYMBOL_TYPE_GHOST)) {
-				item->resolve_parent(*this);
 				auto *parent = const_cast<cache_item *>(item->get_parent(*this));
 
+				if (parent == nullptr) {
+					item->resolve_parent(*this);
+					parent = const_cast<cache_item *>(item->get_parent(*this));
+				}
+
 				if (::fabs(parent->st->weight) < ::fabs(item->st->weight)) {
 					parent->st->weight = item->st->weight;
 				}
diff --git a/src/libserver/symcache/symcache_item.cxx b/src/libserver/symcache/symcache_item.cxx
index dbcd66690..c5751ac35 100644
--- a/src/libserver/symcache/symcache_item.cxx
+++ b/src/libserver/symcache/symcache_item.cxx
@@ -33,6 +33,17 @@ auto cache_item::get_parent(const symcache &cache) const -> const cache_item *
 	return nullptr;
 }
 
+auto cache_item::get_parent_mut(const symcache &cache) -> cache_item *
+{
+	if (is_virtual()) {
+		auto &virtual_sp = std::get<virtual_item>(specific);
+
+		return virtual_sp.get_parent_mut(cache);
+	}
+
+	return nullptr;
+}
+
 auto cache_item::process_deps(const symcache &cache) -> void
 {
 	/* Allow logging macros to work */
@@ -117,12 +128,18 @@ auto cache_item::process_deps(const symcache &cache) -> void
 				}
 				else {
 					/* Create a reverse dep */
-					dit->rdeps.emplace_back(getptr(), dep.sym, id, -1);
-					dep.item = dit->getptr();
-					dep.id = dit->id;
+					if (is_virtual()) {
+						auto *parent = get_parent_mut(cache);
+
+						if (parent) {
+							dit->rdeps.emplace_back(parent->getptr(), dep.sym, parent->id, -1);
+							dep.item = dit->getptr();
+							dep.id = dit->id;
 
-					msg_debug_cache ("add dependency from %d on %d", id,
-							dit->id);
+							msg_debug_cache ("added reverse dependency from %d on %d", id,
+									dit->id);
+						}
+					}
 				}
 			}
 		}
@@ -332,6 +349,15 @@ auto virtual_item::get_parent(const symcache &cache) const -> const cache_item *
 	return cache.get_item_by_id(parent_id, false);
 }
 
+auto virtual_item::get_parent_mut(const symcache &cache) -> cache_item *
+{
+	if (parent) {
+		return parent.get();
+	}
+
+	return const_cast<cache_item *>(cache.get_item_by_id(parent_id, false));
+}
+
 auto virtual_item::resolve_parent(const symcache &cache) -> bool
 {
 	if (parent) {
diff --git a/src/libserver/symcache/symcache_item.hxx b/src/libserver/symcache/symcache_item.hxx
index 62297a7da..e702fa0db 100644
--- a/src/libserver/symcache/symcache_item.hxx
+++ b/src/libserver/symcache/symcache_item.hxx
@@ -113,6 +113,7 @@ public:
 	}
 
 	auto get_parent(const symcache &cache) const -> const cache_item *;
+	auto get_parent_mut(const symcache &cache) -> cache_item *;
 
 	auto resolve_parent(const symcache &cache) -> bool;
 };
@@ -255,6 +256,7 @@ public:
 	}
 
 	auto get_parent(const symcache &cache) const -> const cache_item *;
+	auto get_parent_mut(const symcache &cache) -> cache_item *;
 
 	auto resolve_parent(const symcache &cache) -> bool;
 
diff --git a/src/libserver/symcache/symcache_runtime.cxx b/src/libserver/symcache/symcache_runtime.cxx
index 0805a10d6..acfec5e8a 100644
--- a/src/libserver/symcache/symcache_runtime.cxx
+++ b/src/libserver/symcache/symcache_runtime.cxx
@@ -738,6 +738,7 @@ symcache_runtime::finalize_item(struct rspamd_task *task, cache_item *item) -> v
 		cbd->event = rspamd_session_add_event (task->s,
 				rspamd_symcache_delayed_item_fin, cbd,
 				"symcache");
+		cbd->runtime = this;
 
 		/*
 		 * If no event could be added, then we are already in the destruction