Browse Source

Fix race condition when adding a dependency

gingerBill 4 years ago
parent
commit
94d298755a
3 changed files with 14 additions and 8 deletions
  1. 6 3
      src/check_expr.cpp
  2. 6 4
      src/checker.cpp
  3. 2 1
      src/checker.hpp

+ 6 - 3
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;
 }

+ 6 - 4
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);
 	}
 }
 

+ 2 - 1
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;