From cbfb32c34c09fd13098f0127bc98c88b53587a97 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Tue, 13 Feb 2024 16:21:41 +0000 Subject: Fix race condition with regards to #soa arrays by using the fields mutex --- src/threading.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index c283da425..b8bc9b118 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -119,17 +119,25 @@ struct MutexGuard { explicit MutexGuard(RecursiveMutex *rm) noexcept : rm{rm} { mutex_lock(this->rm); } + explicit MutexGuard(RwMutex *rm) noexcept : rwm{rwm} { + rw_mutex_lock(this->rwm); + } explicit MutexGuard(BlockingMutex &bm) noexcept : bm{&bm} { mutex_lock(this->bm); } explicit MutexGuard(RecursiveMutex &rm) noexcept : rm{&rm} { mutex_lock(this->rm); } + explicit MutexGuard(RwMutex &rwm) noexcept : rwm{&rwm} { + rw_mutex_lock(this->rwm); + } ~MutexGuard() noexcept { if (this->bm) { mutex_unlock(this->bm); } else if (this->rm) { mutex_unlock(this->rm); + } else if (this->rwm) { + rw_mutex_unlock(this->rwm); } } @@ -137,10 +145,12 @@ struct MutexGuard { BlockingMutex *bm; RecursiveMutex *rm; + RwMutex *rwm; }; #define MUTEX_GUARD_BLOCK(m) if (MutexGuard GB_DEFER_3(_mutex_guard_){m}) #define MUTEX_GUARD(m) mutex_lock(m); defer (mutex_unlock(m)) +#define RW_MUTEX_GUARD(m) rw_mutex_lock(m); defer (rw_mutex_unlock(m)) struct RecursiveMutex { -- cgit v1.2.3 From d496dbf3a0ee05819ab6e802939b4219cfa9c7fe Mon Sep 17 00:00:00 2001 From: gingerBill Date: Tue, 13 Feb 2024 16:54:41 +0000 Subject: Fix race condition with #soa --- src/check_type.cpp | 6 ++---- src/threading.cpp | 16 ++++++++++++++++ src/types.cpp | 17 ++++++----------- 3 files changed, 24 insertions(+), 15 deletions(-) (limited to 'src/threading.cpp') diff --git a/src/check_type.cpp b/src/check_type.cpp index 66f8b1185..8a140d95e 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -632,9 +632,6 @@ gb_internal void check_struct_type(CheckerContext *ctx, Type *struct_type, Ast * scope_reserve(ctx->scope, min_field_count); - rw_mutex_lock(&struct_type->Struct.fields_mutex); - defer (rw_mutex_unlock(&struct_type->Struct.fields_mutex)); - if (st->is_raw_union && min_field_count > 1) { struct_type->Struct.is_raw_union = true; context = str_lit("struct #raw_union"); @@ -662,6 +659,7 @@ gb_internal void check_struct_type(CheckerContext *ctx, Type *struct_type, Ast * gb_unused(where_clause_ok); } check_struct_fields(ctx, node, &struct_type->Struct.fields, &struct_type->Struct.tags, st->fields, min_field_count, struct_type, context); + wait_signal_set(&struct_type->Struct.fields_wait_signal); } #define ST_ALIGN(_name) if (st->_name != nullptr) { \ @@ -2553,8 +2551,8 @@ gb_internal Type *make_soa_struct_internal(CheckerContext *ctx, Ast *array_typ_e GB_ASSERT(is_type_struct(elem)); Type *old_struct = base_type(elem); - RW_MUTEX_GUARD(&old_struct->Struct.fields_mutex); + wait_signal_until_available(&old_struct->Struct.fields_wait_signal); field_count = old_struct->Struct.fields.count; soa_struct = alloc_type_struct(); diff --git a/src/threading.cpp b/src/threading.cpp index b8bc9b118..731394126 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -107,6 +107,22 @@ gb_internal void thread_set_name (Thread *t, char const *name); gb_internal void yield_thread(void); gb_internal void yield_process(void); +struct Wait_Signal { + Futex futex; +}; + +gb_internal void wait_signal_until_available(Wait_Signal *ws) { + if (ws->futex.load() == 0) { + futex_wait(&ws->futex, 1); + } +} + +gb_internal void wait_signal_set(Wait_Signal *ws) { + ws->futex.store(1); + futex_broadcast(&ws->futex); +} + + struct MutexGuard { MutexGuard() = delete; diff --git a/src/types.cpp b/src/types.cpp index 04fb06582..2f1994574 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -144,7 +144,7 @@ struct TypeStruct { Type * soa_elem; i32 soa_count; StructSoaKind soa_kind; - RwMutex fields_mutex; + Wait_Signal fields_wait_signal; BlockingMutex offset_mutex; // for settings offsets bool is_polymorphic; @@ -2969,9 +2969,8 @@ gb_internal Selection lookup_field_from_index(Type *type, i64 index) { isize max_count = 0; switch (type->kind) { case Type_Struct: - rw_mutex_shared_lock(&type->Struct.fields_mutex); + wait_signal_until_available(&type->Struct.fields_wait_signal); max_count = type->Struct.fields.count; - rw_mutex_shared_unlock(&type->Struct.fields_mutex); break; case Type_Tuple: max_count = type->Tuple.variables.count; break; } @@ -2982,8 +2981,7 @@ gb_internal Selection lookup_field_from_index(Type *type, i64 index) { switch (type->kind) { case Type_Struct: { - rw_mutex_shared_lock(&type->Struct.fields_mutex); - defer (rw_mutex_shared_unlock(&type->Struct.fields_mutex)); + wait_signal_until_available(&type->Struct.fields_wait_signal); for (isize i = 0; i < max_count; i++) { Entity *f = type->Struct.fields[i]; if (f->kind == Entity_Variable) { @@ -3048,9 +3046,8 @@ gb_internal Selection lookup_field_with_selection(Type *type_, String field_name } } if (type->kind == Type_Struct) { - rw_mutex_shared_lock(&type->Struct.fields_mutex); + wait_signal_until_available(&type->Struct.fields_wait_signal); isize field_count = type->Struct.fields.count; - rw_mutex_shared_unlock(&type->Struct.fields_mutex); if (field_count != 0) for_array(i, type->Struct.fields) { Entity *f = type->Struct.fields[i]; if (f->flags&EntityFlag_Using) { @@ -3079,9 +3076,8 @@ gb_internal Selection lookup_field_with_selection(Type *type_, String field_name } if (type->kind == Type_Struct) { - rw_mutex_shared_lock(&type->Struct.fields_mutex); + wait_signal_until_available(&type->Struct.fields_wait_signal); Scope *s = type->Struct.scope; - rw_mutex_shared_unlock(&type->Struct.fields_mutex); if (s != nullptr) { Entity *found = scope_lookup_current(s, field_name); if (found != nullptr && found->kind != Entity_Variable) { @@ -3129,9 +3125,8 @@ gb_internal Selection lookup_field_with_selection(Type *type_, String field_name } } - rw_mutex_shared_lock(&type->Struct.fields_mutex); + wait_signal_until_available(&type->Struct.fields_wait_signal); isize field_count = type->Struct.fields.count; - rw_mutex_shared_unlock(&type->Struct.fields_mutex); if (field_count != 0) for_array(i, type->Struct.fields) { Entity *f = type->Struct.fields[i]; if (f->kind != Entity_Variable || (f->flags & EntityFlag_Field) == 0) { -- cgit v1.2.3 From c5c2a4d09d98f0d3b6263e204785553e47b83395 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Tue, 13 Feb 2024 17:13:39 +0000 Subject: Fix typo --- src/threading.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index 731394126..725b58c89 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -135,7 +135,7 @@ struct MutexGuard { explicit MutexGuard(RecursiveMutex *rm) noexcept : rm{rm} { mutex_lock(this->rm); } - explicit MutexGuard(RwMutex *rm) noexcept : rwm{rwm} { + explicit MutexGuard(RwMutex *rwm) noexcept : rwm{rwm} { rw_mutex_lock(this->rwm); } explicit MutexGuard(BlockingMutex &bm) noexcept : bm{&bm} { -- cgit v1.2.3 From d7b7804215e451d620894affedc57a42b5105f6b Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 21 Feb 2024 12:55:26 +0000 Subject: `if` -> `while` in `wait_signal_until_available` to check for spurious wake-ups --- src/threading.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index 725b58c89..4f7f5b12b 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -112,7 +112,7 @@ struct Wait_Signal { }; gb_internal void wait_signal_until_available(Wait_Signal *ws) { - if (ws->futex.load() == 0) { + while (ws->futex.load() == 0) { futex_wait(&ws->futex, 1); } } -- cgit v1.2.3 From 21d1c0e5a41627cfc0bc65e00a13d40e7380c50d Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 21 Feb 2024 12:58:26 +0000 Subject: Revert change since it is not needed --- src/threading.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index 4f7f5b12b..725b58c89 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -112,7 +112,7 @@ struct Wait_Signal { }; gb_internal void wait_signal_until_available(Wait_Signal *ws) { - while (ws->futex.load() == 0) { + if (ws->futex.load() == 0) { futex_wait(&ws->futex, 1); } } -- cgit v1.2.3 From fea38f6910f4abb037e2581f28d7592c31991efe Mon Sep 17 00:00:00 2001 From: gingerBill Date: Thu, 22 Feb 2024 14:01:39 +0000 Subject: Minor changes to futex implementation on Linux --- src/check_expr.cpp | 6 ++++-- src/threading.cpp | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'src/threading.cpp') diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 11eb4b533..685bcdd6e 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -1241,7 +1241,7 @@ gb_internal bool is_polymorphic_type_assignable(CheckerContext *c, Type *poly, T } case Type_Pointer: if (source->kind == Type_Pointer) { - isize level = check_is_assignable_to_using_subtype(source->Pointer.elem, poly->Pointer.elem); + isize level = check_is_assignable_to_using_subtype(source->Pointer.elem, poly->Pointer.elem, /*level*/0, /*src_is_ptr*/false, /*allow_polymorphic*/true); if (level > 0) { return true; } @@ -1413,7 +1413,9 @@ gb_internal bool is_polymorphic_type_assignable(CheckerContext *c, Type *poly, T return ok; } - // return check_is_assignable_to(c, &o, poly); + + // NOTE(bill): Check for subtypes of + // return check_is_assignable_to(c, &o, poly); // && is_type_subtype_of_and_allow_polymorphic(o.type, poly); } return false; case Type_Tuple: diff --git a/src/threading.cpp b/src/threading.cpp index 725b58c89..684b13bc3 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -656,7 +656,7 @@ gb_internal void futex_wait(Futex *addr, Footex val) { for (;;) { int ret = syscall(SYS_futex, addr, FUTEX_WAIT | FUTEX_PRIVATE_FLAG, val, NULL, NULL, 0); if (ret == -1) { - if (errno != EAGAIN) { + if (errno != EAGAIN && errno != EINTR) { perror("Futex wait"); GB_PANIC("Failed in futex wait!\n"); } else { -- cgit v1.2.3 From f3b0b82461f45ca6e0bb5e9a06e67cb02662053c Mon Sep 17 00:00:00 2001 From: gingerBill Date: Thu, 22 Feb 2024 14:04:31 +0000 Subject: Fix futex --- src/threading.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index 684b13bc3..725b58c89 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -656,7 +656,7 @@ gb_internal void futex_wait(Futex *addr, Footex val) { for (;;) { int ret = syscall(SYS_futex, addr, FUTEX_WAIT | FUTEX_PRIVATE_FLAG, val, NULL, NULL, 0); if (ret == -1) { - if (errno != EAGAIN && errno != EINTR) { + if (errno != EAGAIN) { perror("Futex wait"); GB_PANIC("Failed in futex wait!\n"); } else { -- cgit v1.2.3