From 94d298755a6036c3dbbf95aed3299e021c3fb9c8 Mon Sep 17 00:00:00 2001
From: gingerBill <bill@gingerbill.org>
Date: Mon, 16 Aug 2021 15:33:26 +0100
Subject: [PATCH] Fix race condition when adding a dependency

---
 src/check_expr.cpp |  9 ++++++---
 src/checker.cpp    | 10 ++++++----
 src/checker.hpp    |  3 ++-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/check_expr.cpp b/src/check_expr.cpp
index 30f44a59f..e0eb5a954 100644
--- a/src/check_expr.cpp
+++ b/src/check_expr.cpp
@@ -4117,20 +4117,23 @@ bool check_identifier_exists(Scope *s, Ast *node, bool nested = false, Scope **o
 }
 
 isize add_dependencies_from_unpacking(CheckerContext *c, Entity **lhs, isize lhs_count, isize tuple_index, isize tuple_count) {
-	if (lhs != nullptr) {
+	if (lhs != nullptr && c->decl != nullptr) {
+		mutex_lock(&c->info->deps_mutex);
+
 		for (isize j = 0; (tuple_index + j) < lhs_count && j < tuple_count; j++) {
 			Entity *e = lhs[tuple_index + j];
 			if (e != nullptr) {
 				DeclInfo *decl = decl_info_of_entity(e);
 				if (decl != nullptr) {
-					c->decl = decl; // will be reset by the 'defer' any way
 					for_array(k, decl->deps.entries) {
 						Entity *dep = decl->deps.entries[k].ptr;
-						add_declaration_dependency(c, dep); // TODO(bill): Should this be here?
+						ptr_set_add(&c->decl->deps, dep);
 					}
 				}
 			}
 		}
+
+		mutex_unlock(&c->info->deps_mutex);
 	}
 	return tuple_count;
 }
diff --git a/src/checker.cpp b/src/checker.cpp
index 79a60f896..11d706403 100644
--- a/src/checker.cpp
+++ b/src/checker.cpp
@@ -629,14 +629,16 @@ void check_scope_usage(Checker *c, Scope *scope) {
 }
 
 
-void add_dependency(DeclInfo *d, Entity *e) {
+void add_dependency(CheckerInfo *info, DeclInfo *d, Entity *e) {
+	mutex_lock(&info->deps_mutex);
 	ptr_set_add(&d->deps, e);
+	mutex_unlock(&info->deps_mutex);
 }
 void add_type_info_dependency(DeclInfo *d, Type *type) {
 	if (d == nullptr) {
-		// GB_ASSERT(type == t_invalid);
 		return;
 	}
+	// NOTE(bill): no mutex is required here because the only procedure calling it is wrapped in a mutex already
 	ptr_set_add(&d->type_info_deps, type);
 }
 
@@ -657,7 +659,7 @@ void add_package_dependency(CheckerContext *c, char const *package_name, char co
 	e->flags |= EntityFlag_Used;
 	GB_ASSERT_MSG(e != nullptr, "%s", name);
 	GB_ASSERT(c->decl != nullptr);
-	ptr_set_add(&c->decl->deps, e);
+	add_dependency(c->info, c->decl, e);
 }
 
 void add_declaration_dependency(CheckerContext *c, Entity *e) {
@@ -665,7 +667,7 @@ void add_declaration_dependency(CheckerContext *c, Entity *e) {
 		return;
 	}
 	if (c->decl != nullptr) {
-		add_dependency(c->decl, e);
+		add_dependency(c->info, c->decl, e);
 	}
 }
 
diff --git a/src/checker.hpp b/src/checker.hpp
index bf462de71..dea24ae53 100644
--- a/src/checker.hpp
+++ b/src/checker.hpp
@@ -300,7 +300,8 @@ struct CheckerInfo {
 	BlockingMutex global_untyped_mutex;
 	BlockingMutex builtin_mutex;
 
-	// NOT recursive & Only used at the end of `check_proc_body`
+	// NOT recursive & only used at the end of `check_proc_body`
+	// and in `add_dependency`.
 	// This is a possible source of contention but probably not
 	// too much of a problem in practice
 	BlockingMutex deps_mutex;