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 c178f7199d7070c5481c5f0f3077f8dcbfa90226 Mon Sep 17 00:00:00 2001 From: Slendi Date: Thu, 15 Feb 2024 15:51:28 +0200 Subject: Get Odin to compile on Haiku This patch makes Odin to compile on Haiku which is a good first step. Now, all that's needed to do is to figure out how to do futexes, which I am blaming for the program crashing. --- build_odin.sh | 5 + src/gb/gb.h | 49 ++- src/path.cpp | 922 +++++++++++++++++++++++++++--------------------------- src/threading.cpp | 47 ++- 4 files changed, 559 insertions(+), 464 deletions(-) (limited to 'src/threading.cpp') diff --git a/build_odin.sh b/build_odin.sh index 589aeb550..0d7750ffa 100755 --- a/build_odin.sh +++ b/build_odin.sh @@ -83,6 +83,11 @@ OpenBSD) LDFLAGS="$LDFLAGS -liconv" LDFLAGS="$LDFLAGS $($LLVM_CONFIG --libs core native --system-libs)" ;; +Haiku) + CXXFLAGS="$CXXFLAGS $($LLVM_CONFIG --cxxflags --ldflags) -I/system/develop/headers/private/shared -I/system/develop/headers/private/kernel" + LDFLAGS="$LDFLAGS -liconv" + LDFLAGS="$LDFLAGS $($LLVM_CONFIG --libs core native --system-libs)" + ;; *) error "Platform \"$OS_NAME\" unsupported" ;; diff --git a/src/gb/gb.h b/src/gb/gb.h index 93d250f21..702647121 100644 --- a/src/gb/gb.h +++ b/src/gb/gb.h @@ -83,6 +83,10 @@ extern "C" { #ifndef GB_SYSTEM_OPENBSD #define GB_SYSTEM_OPENBSD 1 #endif + #elif defined(__HAIKU__) || defined(__haiku__) + #ifndef GB_SYSTEM_HAIKU + #define GB_SYSTEM_HAIKU 1 + #endif #else #error This UNIX operating system is not supported #endif @@ -206,7 +210,7 @@ extern "C" { #endif #include // NOTE(bill): malloc on linux #include - #if !defined(GB_SYSTEM_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) + #if !defined(GB_SYSTEM_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__HAIKU__) #include #endif #include @@ -247,6 +251,13 @@ extern "C" { #include #define lseek64 lseek #endif + +#if defined(GB_SYSTEM_HAIKU) + #include + #include + #include + #define lseek64 lseek +#endif #if defined(GB_SYSTEM_UNIX) #include @@ -801,6 +812,13 @@ typedef struct gbAffinity { isize thread_count; isize threads_per_core; } gbAffinity; +#elif defined(GB_SYSTEM_HAIKU) +typedef struct gbAffinity { + b32 is_accurate; + isize core_count; + isize thread_count; + isize threads_per_core; +} gbAffinity; #else #error TODO(bill): Unknown system #endif @@ -2984,6 +3002,8 @@ gb_inline u32 gb_thread_current_id(void) { __asm__("mov %%fs:0x10,%0" : "=r"(thread_id)); #elif defined(GB_SYSTEM_LINUX) thread_id = gettid(); +#elif defined(GB_SYSTEM_HAIKU) + thread_id = find_thread(NULL); #else #error Unsupported architecture for gb_thread_current_id() #endif @@ -3184,7 +3204,9 @@ b32 gb_affinity_set(gbAffinity *a, isize core, isize thread_index) { //info.affinity_tag = cast(integer_t)index; //result = thread_policy_set(thread, THREAD_AFFINITY_POLICY, cast(thread_policy_t)&info, THREAD_AFFINITY_POLICY_COUNT); +#if !defined(GB_SYSTEM_HAIKU) result = pthread_setaffinity_np(thread, sizeof(cpuset_t), &mn); +#endif return result == 0; } @@ -3236,6 +3258,29 @@ b32 gb_affinity_set(gbAffinity *a, isize core, isize thread_index) { return true; } +isize gb_affinity_thread_count_for_core(gbAffinity *a, isize core) { + GB_ASSERT(0 <= core && core < a->core_count); + return a->threads_per_core; +} +#elif defined(GB_SYSTEM_HAIKU) +#include + +void gb_affinity_init(gbAffinity *a) { + a->core_count = sysconf(_SC_NPROCESSORS_ONLN); + a->threads_per_core = 1; + a->is_accurate = a->core_count > 0; + a->core_count = a->is_accurate ? a->core_count : 1; + a->thread_count = a->core_count; +} + +void gb_affinity_destroy(gbAffinity *a) { + gb_unused(a); +} + +b32 gb_affinity_set(gbAffinity *a, isize core, isize thread_index) { + return true; +} + isize gb_affinity_thread_count_for_core(gbAffinity *a, isize core) { GB_ASSERT(0 <= core && core < a->core_count); return a->threads_per_core; @@ -5457,7 +5502,7 @@ gb_inline b32 gb_file_copy(char const *existing_filename, char const *new_filena } } - gb_free(buf); + gb_mfree(buf); close(new_fd); close(existing_fd); diff --git a/src/path.cpp b/src/path.cpp index de80c9def..742bba7f8 100644 --- a/src/path.cpp +++ b/src/path.cpp @@ -1,461 +1,461 @@ -/* - Path handling utilities. -*/ -#if !defined(GB_SYSTEM_WINDOWS) -#include -#endif - -gb_internal String remove_extension_from_path(String const &s) { - if (s.len != 0 && s.text[s.len-1] == '.') { - return s; - } - for (isize i = s.len-1; i >= 0; i--) { - if (s[i] == '.') { - return substring(s, 0, i); - } - } - return s; -} - -gb_internal String remove_directory_from_path(String const &s) { - isize len = 0; - for (isize i = s.len-1; i >= 0; i--) { - if (s[i] == '/' || - s[i] == '\\') { - break; - } - len += 1; - } - return substring(s, s.len-len, s.len); -} - - -// NOTE(Mark Naughton): getcwd as String -#if !defined(GB_SYSTEM_WINDOWS) -gb_internal String get_current_directory(void) { - char cwd[256]; - getcwd(cwd, 256); - - return make_string_c(cwd); -} - -#else -gb_internal String get_current_directory(void) { - gbAllocator a = heap_allocator(); - - wchar_t cwd[256]; - GetCurrentDirectoryW(256, cwd); - - String16 wstr = make_string16_c(cwd); - - return string16_to_string(a, wstr); -} -#endif - -gb_internal bool path_is_directory(String path); - -gb_internal String directory_from_path(String const &s) { - if (path_is_directory(s)) { - return s; - } - - isize i = s.len-1; - for (; i >= 0; i--) { - if (s[i] == '/' || - s[i] == '\\') { - break; - } - } - if (i >= 0) { - return substring(s, 0, i); - } - return substring(s, 0, 0); -} - -#if defined(GB_SYSTEM_WINDOWS) - gb_internal bool path_is_directory(String path) { - gbAllocator a = heap_allocator(); - String16 wstr = string_to_string16(a, path); - defer (gb_free(a, wstr.text)); - - i32 attribs = GetFileAttributesW(wstr.text); - if (attribs < 0) return false; - - return (attribs & FILE_ATTRIBUTE_DIRECTORY) != 0; - } - -#else - gb_internal bool path_is_directory(String path) { - gbAllocator a = heap_allocator(); - char *copy = cast(char *)copy_string(a, path).text; - defer (gb_free(a, copy)); - - struct stat s; - if (stat(copy, &s) == 0) { - return (s.st_mode & S_IFDIR) != 0; - } - return false; - } -#endif - - -gb_internal String path_to_full_path(gbAllocator a, String path) { - gbAllocator ha = heap_allocator(); - char *path_c = gb_alloc_str_len(ha, cast(char *)path.text, path.len); - defer (gb_free(ha, path_c)); - - char *fullpath = gb_path_get_full_name(a, path_c); - String res = string_trim_whitespace(make_string_c(fullpath)); -#if defined(GB_SYSTEM_WINDOWS) - for (isize i = 0; i < res.len; i++) { - if (res.text[i] == '\\') { - res.text[i] = '/'; - } - } -#endif - return copy_string(a, res); -} - -struct Path { - String basename; - String name; - String ext; -}; - -// NOTE(Jeroen): Naively turns a Path into a string. -gb_internal String path_to_string(gbAllocator a, Path path) { - if (path.basename.len + path.name.len + path.ext.len == 0) { - return make_string(nullptr, 0); - } - - isize len = path.basename.len + 1 + path.name.len + 1; - if (path.ext.len > 0) { - len += path.ext.len + 1; - } - - u8 *str = gb_alloc_array(a, u8, len); - - isize i = 0; - gb_memmove(str+i, path.basename.text, path.basename.len); i += path.basename.len; - - gb_memmove(str+i, "/", 1); i += 1; - - gb_memmove(str+i, path.name.text, path.name.len); i += path.name.len; - if (path.ext.len > 0) { - gb_memmove(str+i, ".", 1); i += 1; - gb_memmove(str+i, path.ext.text, path.ext.len); i += path.ext.len; - } - str[i] = 0; - - String res = make_string(str, i); - res = string_trim_whitespace(res); - return res; -} - -// NOTE(Jeroen): Naively turns a Path into a string, then normalizes it using `path_to_full_path`. -gb_internal String path_to_full_path(gbAllocator a, Path path) { - String temp = path_to_string(heap_allocator(), path); - defer (gb_free(heap_allocator(), temp.text)); - - return path_to_full_path(a, temp); -} - -// NOTE(Jeroen): Takes a path like "odin" or "W:\Odin", turns it into a full path, -// and then breaks it into its components to make a Path. -gb_internal Path path_from_string(gbAllocator a, String const &path) { - Path res = {}; - - if (path.len == 0) return res; - - String fullpath = path_to_full_path(a, path); - defer (gb_free(heap_allocator(), fullpath.text)); - - res.basename = directory_from_path(fullpath); - res.basename = copy_string(a, res.basename); - - if (path_is_directory(fullpath)) { - // It's a directory. We don't need to tinker with the name and extension. - // It could have a superfluous trailing `/`. Remove it if so. - if (res.basename.len > 0 && res.basename.text[res.basename.len - 1] == '/') { - res.basename.len--; - } - return res; - } - - // Note(Dragos): Is the copy_string required if it's a substring? - isize name_start = (res.basename.len > 0) ? res.basename.len + 1 : res.basename.len; - res.name = substring(fullpath, name_start, fullpath.len); - res.name = remove_extension_from_path(res.name); - res.name = copy_string(a, res.name); - - res.ext = path_extension(fullpath, false); // false says not to include the dot. - res.ext = copy_string(a, res.ext); - return res; -} - -// NOTE(Jeroen): Takes a path String and returns the last path element. -gb_internal String last_path_element(String const &path) { - isize count = 0; - u8 * start = (u8 *)(&path.text[path.len - 1]); - for (isize length = path.len; length > 0 && path.text[length - 1] != '/'; length--) { - count++; - start--; - } - if (count > 0) { - start++; // Advance past the `/` and return the substring. - String res = make_string(start, count); - return res; - } - // Must be a root path like `/` or `C:/`, return empty String. - return STR_LIT(""); -} - -gb_internal bool path_is_directory(Path path) { - String path_string = path_to_full_path(heap_allocator(), path); - defer (gb_free(heap_allocator(), path_string.text)); - - return path_is_directory(path_string); -} - -struct FileInfo { - String name; - String fullpath; - i64 size; - bool is_dir; -}; - -enum ReadDirectoryError { - ReadDirectory_None, - - ReadDirectory_InvalidPath, - ReadDirectory_NotExists, - ReadDirectory_Permission, - ReadDirectory_NotDir, - ReadDirectory_Empty, - ReadDirectory_Unknown, - - ReadDirectory_COUNT, -}; - -gb_internal i64 get_file_size(String path) { - char *c_str = alloc_cstring(heap_allocator(), path); - defer (gb_free(heap_allocator(), c_str)); - - gbFile f = {}; - gbFileError err = gb_file_open(&f, c_str); - defer (gb_file_close(&f)); - if (err != gbFileError_None) { - return -1; - } - return gb_file_size(&f); -} - - -#if defined(GB_SYSTEM_WINDOWS) -gb_internal ReadDirectoryError read_directory(String path, Array *fi) { - GB_ASSERT(fi != nullptr); - - - while (path.len > 0) { - Rune end = path[path.len-1]; - if (end == '/') { - path.len -= 1; - } else if (end == '\\') { - path.len -= 1; - } else { - break; - } - } - - if (path.len == 0) { - return ReadDirectory_InvalidPath; - } - { - char *c_str = alloc_cstring(temporary_allocator(), path); - gbFile f = {}; - gbFileError file_err = gb_file_open(&f, c_str); - defer (gb_file_close(&f)); - - switch (file_err) { - case gbFileError_Invalid: return ReadDirectory_InvalidPath; - case gbFileError_NotExists: return ReadDirectory_NotExists; - // case gbFileError_Permission: return ReadDirectory_Permission; - } - } - - if (!path_is_directory(path)) { - return ReadDirectory_NotDir; - } - - - gbAllocator a = heap_allocator(); - char *new_path = gb_alloc_array(a, char, path.len+3); - defer (gb_free(a, new_path)); - - gb_memmove(new_path, path.text, path.len); - gb_memmove(new_path+path.len, "/*", 2); - new_path[path.len+2] = 0; - - String np = make_string(cast(u8 *)new_path, path.len+2); - String16 wstr = string_to_string16(a, np); - defer (gb_free(a, wstr.text)); - - WIN32_FIND_DATAW file_data = {}; - HANDLE find_file = FindFirstFileW(wstr.text, &file_data); - if (find_file == INVALID_HANDLE_VALUE) { - return ReadDirectory_Unknown; - } - defer (FindClose(find_file)); - - array_init(fi, a, 0, 100); - - do { - wchar_t *filename_w = file_data.cFileName; - u64 size = cast(u64)file_data.nFileSizeLow; - size |= (cast(u64)file_data.nFileSizeHigh) << 32; - String name = string16_to_string(a, make_string16_c(filename_w)); - if (name == "." || name == "..") { - gb_free(a, name.text); - continue; - } - - String filepath = {}; - filepath.len = path.len+1+name.len; - filepath.text = gb_alloc_array(a, u8, filepath.len+1); - defer (gb_free(a, filepath.text)); - gb_memmove(filepath.text, path.text, path.len); - gb_memmove(filepath.text+path.len, "/", 1); - gb_memmove(filepath.text+path.len+1, name.text, name.len); - - FileInfo info = {}; - info.name = name; - info.fullpath = path_to_full_path(a, filepath); - info.size = cast(i64)size; - info.is_dir = (file_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; - array_add(fi, info); - } while (FindNextFileW(find_file, &file_data)); - - if (fi->count == 0) { - return ReadDirectory_Empty; - } - - return ReadDirectory_None; -} -#elif defined(GB_SYSTEM_LINUX) || defined(GB_SYSTEM_OSX) || defined(GB_SYSTEM_FREEBSD) || defined(GB_SYSTEM_OPENBSD) - -#include - -gb_internal ReadDirectoryError read_directory(String path, Array *fi) { - GB_ASSERT(fi != nullptr); - - gbAllocator a = heap_allocator(); - - char *c_path = alloc_cstring(a, path); - defer (gb_free(a, c_path)); - - DIR *dir = opendir(c_path); - if (!dir) { - switch (errno) { - case ENOENT: - return ReadDirectory_NotExists; - case EACCES: - return ReadDirectory_Permission; - case ENOTDIR: - return ReadDirectory_NotDir; - default: - // ENOMEM: out of memory - // EMFILE: per-process limit on open fds reached - // ENFILE: system-wide limit on total open files reached - return ReadDirectory_Unknown; - } - GB_PANIC("unreachable"); - } - - array_init(fi, a, 0, 100); - - for (;;) { - struct dirent *entry = readdir(dir); - if (entry == nullptr) { - break; - } - - String name = make_string_c(entry->d_name); - if (name == "." || name == "..") { - continue; - } - - String filepath = {}; - filepath.len = path.len+1+name.len; - filepath.text = gb_alloc_array(a, u8, filepath.len+1); - defer (gb_free(a, filepath.text)); - gb_memmove(filepath.text, path.text, path.len); - gb_memmove(filepath.text+path.len, "/", 1); - gb_memmove(filepath.text+path.len+1, name.text, name.len); - filepath.text[filepath.len] = 0; - - - struct stat dir_stat = {}; - - if (stat((char *)filepath.text, &dir_stat)) { - continue; - } - - if (S_ISDIR(dir_stat.st_mode)) { - continue; - } - - i64 size = dir_stat.st_size; - - FileInfo info = {}; - info.name = name; - info.fullpath = path_to_full_path(a, filepath); - info.size = size; - array_add(fi, info); - } - - if (fi->count == 0) { - return ReadDirectory_Empty; - } - - return ReadDirectory_None; -} - - -#else -#error Implement read_directory -#endif - -#if !defined(GB_SYSTEM_WINDOWS) -gb_internal bool write_directory(String path) { - char const *pathname = (char *) path.text; - - if (access(pathname, W_OK) < 0) { - return false; - } - - return true; -} -#else -gb_internal bool write_directory(String path) { - String16 wstr = string_to_string16(heap_allocator(), path); - LPCWSTR wdirectory_name = wstr.text; - - HANDLE directory = CreateFileW(wdirectory_name, - GENERIC_WRITE, - 0, - NULL, - OPEN_EXISTING, - FILE_FLAG_BACKUP_SEMANTICS, - NULL); - - if (directory == INVALID_HANDLE_VALUE) { - DWORD error_code = GetLastError(); - if (error_code == ERROR_ACCESS_DENIED) { - return false; - } - } - - CloseHandle(directory); - return true; -} -#endif +/* + Path handling utilities. +*/ +#if !defined(GB_SYSTEM_WINDOWS) +#include +#endif + +gb_internal String remove_extension_from_path(String const &s) { + if (s.len != 0 && s.text[s.len-1] == '.') { + return s; + } + for (isize i = s.len-1; i >= 0; i--) { + if (s[i] == '.') { + return substring(s, 0, i); + } + } + return s; +} + +gb_internal String remove_directory_from_path(String const &s) { + isize len = 0; + for (isize i = s.len-1; i >= 0; i--) { + if (s[i] == '/' || + s[i] == '\\') { + break; + } + len += 1; + } + return substring(s, s.len-len, s.len); +} + + +// NOTE(Mark Naughton): getcwd as String +#if !defined(GB_SYSTEM_WINDOWS) +gb_internal String get_current_directory(void) { + char cwd[256]; + getcwd(cwd, 256); + + return make_string_c(cwd); +} + +#else +gb_internal String get_current_directory(void) { + gbAllocator a = heap_allocator(); + + wchar_t cwd[256]; + GetCurrentDirectoryW(256, cwd); + + String16 wstr = make_string16_c(cwd); + + return string16_to_string(a, wstr); +} +#endif + +gb_internal bool path_is_directory(String path); + +gb_internal String directory_from_path(String const &s) { + if (path_is_directory(s)) { + return s; + } + + isize i = s.len-1; + for (; i >= 0; i--) { + if (s[i] == '/' || + s[i] == '\\') { + break; + } + } + if (i >= 0) { + return substring(s, 0, i); + } + return substring(s, 0, 0); +} + +#if defined(GB_SYSTEM_WINDOWS) + gb_internal bool path_is_directory(String path) { + gbAllocator a = heap_allocator(); + String16 wstr = string_to_string16(a, path); + defer (gb_free(a, wstr.text)); + + i32 attribs = GetFileAttributesW(wstr.text); + if (attribs < 0) return false; + + return (attribs & FILE_ATTRIBUTE_DIRECTORY) != 0; + } + +#else + gb_internal bool path_is_directory(String path) { + gbAllocator a = heap_allocator(); + char *copy = cast(char *)copy_string(a, path).text; + defer (gb_free(a, copy)); + + struct stat s; + if (stat(copy, &s) == 0) { + return (s.st_mode & S_IFDIR) != 0; + } + return false; + } +#endif + + +gb_internal String path_to_full_path(gbAllocator a, String path) { + gbAllocator ha = heap_allocator(); + char *path_c = gb_alloc_str_len(ha, cast(char *)path.text, path.len); + defer (gb_free(ha, path_c)); + + char *fullpath = gb_path_get_full_name(a, path_c); + String res = string_trim_whitespace(make_string_c(fullpath)); +#if defined(GB_SYSTEM_WINDOWS) + for (isize i = 0; i < res.len; i++) { + if (res.text[i] == '\\') { + res.text[i] = '/'; + } + } +#endif + return copy_string(a, res); +} + +struct Path { + String basename; + String name; + String ext; +}; + +// NOTE(Jeroen): Naively turns a Path into a string. +gb_internal String path_to_string(gbAllocator a, Path path) { + if (path.basename.len + path.name.len + path.ext.len == 0) { + return make_string(nullptr, 0); + } + + isize len = path.basename.len + 1 + path.name.len + 1; + if (path.ext.len > 0) { + len += path.ext.len + 1; + } + + u8 *str = gb_alloc_array(a, u8, len); + + isize i = 0; + gb_memmove(str+i, path.basename.text, path.basename.len); i += path.basename.len; + + gb_memmove(str+i, "/", 1); i += 1; + + gb_memmove(str+i, path.name.text, path.name.len); i += path.name.len; + if (path.ext.len > 0) { + gb_memmove(str+i, ".", 1); i += 1; + gb_memmove(str+i, path.ext.text, path.ext.len); i += path.ext.len; + } + str[i] = 0; + + String res = make_string(str, i); + res = string_trim_whitespace(res); + return res; +} + +// NOTE(Jeroen): Naively turns a Path into a string, then normalizes it using `path_to_full_path`. +gb_internal String path_to_full_path(gbAllocator a, Path path) { + String temp = path_to_string(heap_allocator(), path); + defer (gb_free(heap_allocator(), temp.text)); + + return path_to_full_path(a, temp); +} + +// NOTE(Jeroen): Takes a path like "odin" or "W:\Odin", turns it into a full path, +// and then breaks it into its components to make a Path. +gb_internal Path path_from_string(gbAllocator a, String const &path) { + Path res = {}; + + if (path.len == 0) return res; + + String fullpath = path_to_full_path(a, path); + defer (gb_free(heap_allocator(), fullpath.text)); + + res.basename = directory_from_path(fullpath); + res.basename = copy_string(a, res.basename); + + if (path_is_directory(fullpath)) { + // It's a directory. We don't need to tinker with the name and extension. + // It could have a superfluous trailing `/`. Remove it if so. + if (res.basename.len > 0 && res.basename.text[res.basename.len - 1] == '/') { + res.basename.len--; + } + return res; + } + + // Note(Dragos): Is the copy_string required if it's a substring? + isize name_start = (res.basename.len > 0) ? res.basename.len + 1 : res.basename.len; + res.name = substring(fullpath, name_start, fullpath.len); + res.name = remove_extension_from_path(res.name); + res.name = copy_string(a, res.name); + + res.ext = path_extension(fullpath, false); // false says not to include the dot. + res.ext = copy_string(a, res.ext); + return res; +} + +// NOTE(Jeroen): Takes a path String and returns the last path element. +gb_internal String last_path_element(String const &path) { + isize count = 0; + u8 * start = (u8 *)(&path.text[path.len - 1]); + for (isize length = path.len; length > 0 && path.text[length - 1] != '/'; length--) { + count++; + start--; + } + if (count > 0) { + start++; // Advance past the `/` and return the substring. + String res = make_string(start, count); + return res; + } + // Must be a root path like `/` or `C:/`, return empty String. + return STR_LIT(""); +} + +gb_internal bool path_is_directory(Path path) { + String path_string = path_to_full_path(heap_allocator(), path); + defer (gb_free(heap_allocator(), path_string.text)); + + return path_is_directory(path_string); +} + +struct FileInfo { + String name; + String fullpath; + i64 size; + bool is_dir; +}; + +enum ReadDirectoryError { + ReadDirectory_None, + + ReadDirectory_InvalidPath, + ReadDirectory_NotExists, + ReadDirectory_Permission, + ReadDirectory_NotDir, + ReadDirectory_Empty, + ReadDirectory_Unknown, + + ReadDirectory_COUNT, +}; + +gb_internal i64 get_file_size(String path) { + char *c_str = alloc_cstring(heap_allocator(), path); + defer (gb_free(heap_allocator(), c_str)); + + gbFile f = {}; + gbFileError err = gb_file_open(&f, c_str); + defer (gb_file_close(&f)); + if (err != gbFileError_None) { + return -1; + } + return gb_file_size(&f); +} + + +#if defined(GB_SYSTEM_WINDOWS) +gb_internal ReadDirectoryError read_directory(String path, Array *fi) { + GB_ASSERT(fi != nullptr); + + + while (path.len > 0) { + Rune end = path[path.len-1]; + if (end == '/') { + path.len -= 1; + } else if (end == '\\') { + path.len -= 1; + } else { + break; + } + } + + if (path.len == 0) { + return ReadDirectory_InvalidPath; + } + { + char *c_str = alloc_cstring(temporary_allocator(), path); + gbFile f = {}; + gbFileError file_err = gb_file_open(&f, c_str); + defer (gb_file_close(&f)); + + switch (file_err) { + case gbFileError_Invalid: return ReadDirectory_InvalidPath; + case gbFileError_NotExists: return ReadDirectory_NotExists; + // case gbFileError_Permission: return ReadDirectory_Permission; + } + } + + if (!path_is_directory(path)) { + return ReadDirectory_NotDir; + } + + + gbAllocator a = heap_allocator(); + char *new_path = gb_alloc_array(a, char, path.len+3); + defer (gb_free(a, new_path)); + + gb_memmove(new_path, path.text, path.len); + gb_memmove(new_path+path.len, "/*", 2); + new_path[path.len+2] = 0; + + String np = make_string(cast(u8 *)new_path, path.len+2); + String16 wstr = string_to_string16(a, np); + defer (gb_free(a, wstr.text)); + + WIN32_FIND_DATAW file_data = {}; + HANDLE find_file = FindFirstFileW(wstr.text, &file_data); + if (find_file == INVALID_HANDLE_VALUE) { + return ReadDirectory_Unknown; + } + defer (FindClose(find_file)); + + array_init(fi, a, 0, 100); + + do { + wchar_t *filename_w = file_data.cFileName; + u64 size = cast(u64)file_data.nFileSizeLow; + size |= (cast(u64)file_data.nFileSizeHigh) << 32; + String name = string16_to_string(a, make_string16_c(filename_w)); + if (name == "." || name == "..") { + gb_free(a, name.text); + continue; + } + + String filepath = {}; + filepath.len = path.len+1+name.len; + filepath.text = gb_alloc_array(a, u8, filepath.len+1); + defer (gb_free(a, filepath.text)); + gb_memmove(filepath.text, path.text, path.len); + gb_memmove(filepath.text+path.len, "/", 1); + gb_memmove(filepath.text+path.len+1, name.text, name.len); + + FileInfo info = {}; + info.name = name; + info.fullpath = path_to_full_path(a, filepath); + info.size = cast(i64)size; + info.is_dir = (file_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; + array_add(fi, info); + } while (FindNextFileW(find_file, &file_data)); + + if (fi->count == 0) { + return ReadDirectory_Empty; + } + + return ReadDirectory_None; +} +#elif defined(GB_SYSTEM_LINUX) || defined(GB_SYSTEM_OSX) || defined(GB_SYSTEM_FREEBSD) || defined(GB_SYSTEM_OPENBSD) || defined(GB_SYSTEM_HAIKU) + +#include + +gb_internal ReadDirectoryError read_directory(String path, Array *fi) { + GB_ASSERT(fi != nullptr); + + gbAllocator a = heap_allocator(); + + char *c_path = alloc_cstring(a, path); + defer (gb_free(a, c_path)); + + DIR *dir = opendir(c_path); + if (!dir) { + switch (errno) { + case ENOENT: + return ReadDirectory_NotExists; + case EACCES: + return ReadDirectory_Permission; + case ENOTDIR: + return ReadDirectory_NotDir; + default: + // ENOMEM: out of memory + // EMFILE: per-process limit on open fds reached + // ENFILE: system-wide limit on total open files reached + return ReadDirectory_Unknown; + } + GB_PANIC("unreachable"); + } + + array_init(fi, a, 0, 100); + + for (;;) { + struct dirent *entry = readdir(dir); + if (entry == nullptr) { + break; + } + + String name = make_string_c(entry->d_name); + if (name == "." || name == "..") { + continue; + } + + String filepath = {}; + filepath.len = path.len+1+name.len; + filepath.text = gb_alloc_array(a, u8, filepath.len+1); + defer (gb_free(a, filepath.text)); + gb_memmove(filepath.text, path.text, path.len); + gb_memmove(filepath.text+path.len, "/", 1); + gb_memmove(filepath.text+path.len+1, name.text, name.len); + filepath.text[filepath.len] = 0; + + + struct stat dir_stat = {}; + + if (stat((char *)filepath.text, &dir_stat)) { + continue; + } + + if (S_ISDIR(dir_stat.st_mode)) { + continue; + } + + i64 size = dir_stat.st_size; + + FileInfo info = {}; + info.name = name; + info.fullpath = path_to_full_path(a, filepath); + info.size = size; + array_add(fi, info); + } + + if (fi->count == 0) { + return ReadDirectory_Empty; + } + + return ReadDirectory_None; +} + + +#else +#error Implement read_directory +#endif + +#if !defined(GB_SYSTEM_WINDOWS) +gb_internal bool write_directory(String path) { + char const *pathname = (char *) path.text; + + if (access(pathname, W_OK) < 0) { + return false; + } + + return true; +} +#else +gb_internal bool write_directory(String path) { + String16 wstr = string_to_string16(heap_allocator(), path); + LPCWSTR wdirectory_name = wstr.text; + + HANDLE directory = CreateFileW(wdirectory_name, + GENERIC_WRITE, + 0, + NULL, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + NULL); + + if (directory == INVALID_HANDLE_VALUE) { + DWORD error_code = GetLastError(); + if (error_code == ERROR_ACCESS_DENIED) { + return false; + } + } + + CloseHandle(directory); + return true; +} +#endif diff --git a/src/threading.cpp b/src/threading.cpp index 725b58c89..ea987890b 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -831,8 +831,53 @@ gb_internal void futex_wait(Futex *f, Footex val) { WaitOnAddress(f, (void *)&val, sizeof(val), INFINITE); } while (f->load() == val); } +#elif defined(GB_SYSTEM_HAIKU) + +#include +#include +#include + +struct MutexCond { + pthread_mutex_t mutex; + pthread_cond_t cond; +}; + +std::unordered_map> futex_map; + +MutexCond* get_mutex_cond(Futex* f) { + if (futex_map.find(f) == futex_map.end()) { + futex_map[f] = std::make_unique(); + pthread_mutex_init(&futex_map[f]->mutex, NULL); + pthread_cond_init(&futex_map[f]->cond, NULL); + } + return futex_map[f].get(); +} + +void futex_signal(Futex *f) { + MutexCond* mc = get_mutex_cond(f); + pthread_mutex_lock(&mc->mutex); + pthread_cond_signal(&mc->cond); + pthread_mutex_unlock(&mc->mutex); +} + +void futex_broadcast(Futex *f) { + MutexCond* mc = get_mutex_cond(f); + pthread_mutex_lock(&mc->mutex); + pthread_cond_broadcast(&mc->cond); + pthread_mutex_unlock(&mc->mutex); +} + +void futex_wait(Futex *f, Footex val) { + MutexCond* mc = get_mutex_cond(f); + pthread_mutex_lock(&mc->mutex); + while (f->load() == val) { + pthread_cond_wait(&mc->cond, &mc->mutex); + } + pthread_mutex_unlock(&mc->mutex); +} + #endif #if defined(GB_SYSTEM_WINDOWS) #pragma warning(pop) -#endif \ No newline at end of file +#endif -- cgit v1.2.3 From 824c831190da1efc351de38743876376a5bb1b08 Mon Sep 17 00:00:00 2001 From: avanspector Date: Sat, 24 Feb 2024 23:46:55 +0100 Subject: Implement futex --- src/threading.cpp | 132 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 99 insertions(+), 33 deletions(-) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index ea987890b..1805e51e2 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -831,49 +831,115 @@ gb_internal void futex_wait(Futex *f, Footex val) { WaitOnAddress(f, (void *)&val, sizeof(val), INFINITE); } while (f->load() == val); } + #elif defined(GB_SYSTEM_HAIKU) -#include -#include -#include +// Futex implementation taken from https://tavianator.com/2023/futex.html -struct MutexCond { - pthread_mutex_t mutex; - pthread_cond_t cond; +#include +#include + +struct Futex_Wait_Node { + pthread_t thread; + Futex *futex; + Futex_Wait_Node *prev, *next; }; - -std::unordered_map> futex_map; - -MutexCond* get_mutex_cond(Futex* f) { - if (futex_map.find(f) == futex_map.end()) { - futex_map[f] = std::make_unique(); - pthread_mutex_init(&futex_map[f]->mutex, NULL); - pthread_cond_init(&futex_map[f]->cond, NULL); + +struct Futex_Wait_Queue { + std::atomic_flag spinlock; + Futex_Wait_Node list; + + void lock() { + while (spinlock.test_and_set(std::memory_order_acquire)) { + ; // spin... + } + } + + void unlock() { + spinlock.clear(std::memory_order_release); } - return futex_map[f].get(); -} +}; +// FIXME: This approach may scale badly in the future, +// possible solution - hash map (leads to deadlocks now). + +Futex_Wait_Queue g_waitq = { + .spinlock = ATOMIC_FLAG_INIT, + .list = { + .prev = &g_waitq.list, + .next = &g_waitq.list, + }, +}; + +Futex_Wait_Queue *get_wait_queue(Futex *f) { + // Future hash map method... + return &g_waitq; +} + void futex_signal(Futex *f) { - MutexCond* mc = get_mutex_cond(f); - pthread_mutex_lock(&mc->mutex); - pthread_cond_signal(&mc->cond); - pthread_mutex_unlock(&mc->mutex); + auto waitq = get_wait_queue(f); + + waitq->lock(); + + auto head = &waitq->list; + for (auto waiter = head->next; waiter != head; waiter = waiter->next) { + if (waiter->futex == f) { + pthread_kill(waiter->thread, SIGCONT); + break; + } + } + + waitq->unlock(); } - + void futex_broadcast(Futex *f) { - MutexCond* mc = get_mutex_cond(f); - pthread_mutex_lock(&mc->mutex); - pthread_cond_broadcast(&mc->cond); - pthread_mutex_unlock(&mc->mutex); -} - + auto waitq = get_wait_queue(f); + + waitq->lock(); + + auto head = &waitq->list; + for (auto waiter = head->next; waiter != head; waiter = waiter->next) { + if (waiter->futex == f) { + pthread_kill(waiter->thread, SIGCONT); + } + } + + waitq->unlock(); +} + void futex_wait(Futex *f, Footex val) { - MutexCond* mc = get_mutex_cond(f); - pthread_mutex_lock(&mc->mutex); - while (f->load() == val) { - pthread_cond_wait(&mc->cond, &mc->mutex); - } - pthread_mutex_unlock(&mc->mutex); + auto waitq = get_wait_queue(f); + + waitq->lock(); + + auto head = &waitq->list; + Wait_Node waiter; + waiter.thread = pthread_self(); + waiter.futex = f; + waiter.prev = head; + waiter.next = head->next; + + waiter.prev->next = &waiter; + waiter.next->prev = &waiter; + + sigset_t old_mask, mask; + sigemptyset(&mask); + sigaddset(&mask, SIGCONT); + pthread_sigmask(SIG_BLOCK, &mask, &old_mask); + + if (*f == val) { + waitq->unlock(); + int sig; + sigwait(&mask, &sig); + waitq->lock(); + } + + waiter.prev->next = waiter.next; + waiter.next->prev = waiter.prev; + + pthread_sigmask(SIG_SETMASK, &old_mask, NULL); + + waitq->unlock(); } #endif -- cgit v1.2.3 From 028a79e66c714180852c94db165de3aaa97c1df8 Mon Sep 17 00:00:00 2001 From: avanspector Date: Sun, 25 Feb 2024 02:34:41 +0100 Subject: Update threading.cpp --- src/threading.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index 1805e51e2..6602bf67e 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -492,6 +492,8 @@ gb_internal u32 thread_current_id(void) { __asm__("mov %%fs:0x10,%0" : "=r"(thread_id)); #elif defined(GB_SYSTEM_LINUX) thread_id = gettid(); +#elif defined(GB_SYSTEM_HAIKU) + thread_id = find_thread(NULL); #else #error Unsupported architecture for thread_current_id() #endif -- cgit v1.2.3 From 24c8b1540920bd181dc399bf86f2ec3a8ea72762 Mon Sep 17 00:00:00 2001 From: avanspector Date: Sun, 25 Feb 2024 02:38:35 +0100 Subject: small fixes --- src/build_settings.cpp | 2 ++ src/threading.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'src/threading.cpp') diff --git a/src/build_settings.cpp b/src/build_settings.cpp index f395cb515..e4e360270 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -885,6 +885,8 @@ gb_internal String internal_odin_root_dir(void) { #include +gb_internal String path_to_fullpath(gbAllocator a, String s, bool *ok_); + gb_internal String internal_odin_root_dir(void) { String path = global_module_path; isize len, i; diff --git a/src/threading.cpp b/src/threading.cpp index 6602bf67e..56f246955 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -915,7 +915,7 @@ void futex_wait(Futex *f, Footex val) { waitq->lock(); auto head = &waitq->list; - Wait_Node waiter; + Futex_Wait_Node waiter; waiter.thread = pthread_self(); waiter.futex = f; waiter.prev = head; -- cgit v1.2.3 From d4d9f55556ffa71e519ffcc5df431edc097746e2 Mon Sep 17 00:00:00 2001 From: avanspector Date: Fri, 1 Mar 2024 00:41:28 +0100 Subject: Update threading.cpp --- src/threading.cpp | 145 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 102 insertions(+), 43 deletions(-) (limited to 'src/threading.cpp') diff --git a/src/threading.cpp b/src/threading.cpp index 56f246955..a469435d2 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -840,86 +840,131 @@ gb_internal void futex_wait(Futex *f, Footex val) { #include #include + +struct _Spinlock { + std::atomic_flag state; + + void init() { + state.clear(); + } + + void lock() { + while (state.test_and_set(std::memory_order_acquire)) { + #if defined(GB_CPU_X86) + _mm_pause(); + #else + (void)0; // spin... + #endif + } + } + + void unlock() { + state.clear(std::memory_order_release); + } +}; + +struct Futex_Waitq; -struct Futex_Wait_Node { +struct Futex_Waiter { + _Spinlock lock; pthread_t thread; Futex *futex; - Futex_Wait_Node *prev, *next; + Futex_Waitq *waitq; + Futex_Waiter *prev, *next; }; -struct Futex_Wait_Queue { - std::atomic_flag spinlock; - Futex_Wait_Node list; +struct Futex_Waitq { + _Spinlock lock; + Futex_Waiter list; - void lock() { - while (spinlock.test_and_set(std::memory_order_acquire)) { - ; // spin... - } - } - - void unlock() { - spinlock.clear(std::memory_order_release); + void init() { + auto head = &list; + head->prev = head->next = head; } }; // FIXME: This approach may scale badly in the future, // possible solution - hash map (leads to deadlocks now). -Futex_Wait_Queue g_waitq = { - .spinlock = ATOMIC_FLAG_INIT, +Futex_Waitq g_waitq = { + .lock = ATOMIC_FLAG_INIT, .list = { .prev = &g_waitq.list, .next = &g_waitq.list, }, }; -Futex_Wait_Queue *get_wait_queue(Futex *f) { +Futex_Waitq *get_waitq(Futex *f) { // Future hash map method... return &g_waitq; } void futex_signal(Futex *f) { - auto waitq = get_wait_queue(f); + auto waitq = get_waitq(f); - waitq->lock(); + waitq->lock.lock(); auto head = &waitq->list; for (auto waiter = head->next; waiter != head; waiter = waiter->next) { - if (waiter->futex == f) { - pthread_kill(waiter->thread, SIGCONT); - break; - } + if (waiter->futex != f) { + continue; + } + waitq->lock.unlock(); + pthread_kill(waiter->thread, SIGCONT); + return; } - waitq->unlock(); + waitq->lock.unlock(); } void futex_broadcast(Futex *f) { - auto waitq = get_wait_queue(f); + auto waitq = get_waitq(f); - waitq->lock(); + waitq->lock.lock(); auto head = &waitq->list; for (auto waiter = head->next; waiter != head; waiter = waiter->next) { - if (waiter->futex == f) { + if (waiter->futex != f) { + continue; + } + if (waiter->next == head) { + waitq->lock.unlock(); pthread_kill(waiter->thread, SIGCONT); - } + return; + } else { + pthread_kill(waiter->thread, SIGCONT); + } } - waitq->unlock(); + waitq->lock.unlock(); } void futex_wait(Futex *f, Footex val) { - auto waitq = get_wait_queue(f); - - waitq->lock(); - - auto head = &waitq->list; - Futex_Wait_Node waiter; + Futex_Waiter waiter; waiter.thread = pthread_self(); waiter.futex = f; - waiter.prev = head; - waiter.next = head->next; + + auto waitq = get_waitq(f); + while (waitq->lock.state.test_and_set(std::memory_order_acquire)) { + if (f->load(std::memory_order_relaxed) != val) { + return; + } + #if defined(GB_CPU_X86) + _mm_pause(); + #else + (void)0; // spin... + #endif + } + + waiter.waitq = waitq; + waiter.lock.init(); + waiter.lock.lock(); + + auto head = &waitq->list; + waiter.prev = head->prev; + waiter.next = head; + waiter.prev->next = &waiter; + waiter.next->prev = &waiter; waiter.prev->next = &waiter; waiter.next->prev = &waiter; @@ -928,12 +973,25 @@ void futex_wait(Futex *f, Footex val) { sigemptyset(&mask); sigaddset(&mask, SIGCONT); pthread_sigmask(SIG_BLOCK, &mask, &old_mask); - - if (*f == val) { - waitq->unlock(); - int sig; - sigwait(&mask, &sig); - waitq->lock(); + + if (f->load(std::memory_order_relaxed) == val) { + waiter.lock.unlock(); + waitq->lock.unlock(); + + int sig; + sigwait(&mask, &sig); + + waitq->lock.lock(); + waiter.lock.lock(); + + while (waitq != waiter.waitq) { + auto req = waiter.waitq; + waiter.lock.unlock(); + waitq->lock.unlock(); + waitq = req; + waitq->lock.lock(); + waiter.lock.lock(); + } } waiter.prev->next = waiter.next; @@ -941,7 +999,8 @@ void futex_wait(Futex *f, Footex val) { pthread_sigmask(SIG_SETMASK, &old_mask, NULL); - waitq->unlock(); + waiter.lock.unlock(); + waitq->lock.unlock(); } #endif -- cgit v1.2.3 From 9c455b22130d175bac13fb931de08d7ab09308af Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Fri, 15 Mar 2024 21:10:11 +0100 Subject: darwin: use new wait on address API if possible --- core/sync/futex_darwin.odin | 64 ++++++++- core/sys/darwin/darwin.odin | 9 ++ core/sys/darwin/sync.odin | 309 ++++++++++++++++++++++++++++++++++++++++++++ src/checker.cpp | 9 ++ src/threading.cpp | 69 ++++++++++ 5 files changed, 458 insertions(+), 2 deletions(-) create mode 100644 core/sys/darwin/sync.odin (limited to 'src/threading.cpp') diff --git a/core/sync/futex_darwin.odin b/core/sync/futex_darwin.odin index 44746e57b..6ea177d1b 100644 --- a/core/sync/futex_darwin.odin +++ b/core/sync/futex_darwin.odin @@ -3,6 +3,7 @@ package sync import "core:c" +import "core:sys/darwin" import "core:time" foreign import System "system:System.framework" @@ -29,8 +30,29 @@ _futex_wait :: proc "contextless" (f: ^Futex, expected: u32) -> bool { } _futex_wait_with_timeout :: proc "contextless" (f: ^Futex, expected: u32, duration: time.Duration) -> bool { + when darwin.WAIT_ON_ADDRESS_AVAILABLE { + s: i32 + if duration > 0 { + s = darwin.os_sync_wait_on_address_with_timeout(f, u64(expected), size_of(Futex), {}, .MACH_ABSOLUTE_TIME, u64(duration)) + } else { + s = darwin.os_sync_wait_on_address(f, u64(expected), size_of(Futex), {}) + } + + if s >= 0 { + return true + } + + switch darwin.errno() { + case -EINTR, -EFAULT: + return true + case -ETIMEDOUT: + return false + case: + _panic("darwin.os_sync_wait_on_address_with_timeout failure") + } + } else { + timeout_ns := u32(duration) * 1000 - s := __ulock_wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, u64(expected), timeout_ns) if s >= 0 { return true @@ -45,9 +67,27 @@ _futex_wait_with_timeout :: proc "contextless" (f: ^Futex, expected: u32, durati } return true + } } _futex_signal :: proc "contextless" (f: ^Futex) { + when darwin.WAIT_ON_ADDRESS_AVAILABLE { + loop: for { + s := darwin.os_sync_wake_by_address_any(f, size_of(Futex), {}) + if s >= 0 { + return + } + switch darwin.errno() { + case -EINTR, -EFAULT: + continue loop + case -ENOENT: + return + case: + _panic("darwin.os_sync_wake_by_address_any failure") + } + } + } else { + loop: for { s := __ulock_wake(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, 0) if s >= 0 { @@ -62,9 +102,28 @@ _futex_signal :: proc "contextless" (f: ^Futex) { _panic("futex_wake_single failure") } } + + } } _futex_broadcast :: proc "contextless" (f: ^Futex) { + when darwin.WAIT_ON_ADDRESS_AVAILABLE { + loop: for { + s := darwin.os_sync_wake_by_address_all(f, size_of(Futex), {}) + if s >= 0 { + return + } + switch darwin.errno() { + case -EINTR, -EFAULT: + continue loop + case -ENOENT: + return + case: + _panic("darwin.os_sync_wake_by_address_all failure") + } + } + } else { + loop: for { s := __ulock_wake(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO | ULF_WAKE_ALL, f, 0) if s >= 0 { @@ -79,5 +138,6 @@ _futex_broadcast :: proc "contextless" (f: ^Futex) { _panic("futex_wake_all failure") } } -} + } +} diff --git a/core/sys/darwin/darwin.odin b/core/sys/darwin/darwin.odin index a3e07277c..ddd25a76c 100644 --- a/core/sys/darwin/darwin.odin +++ b/core/sys/darwin/darwin.odin @@ -3,6 +3,8 @@ package darwin import "core:c" +foreign import system "system:System.framework" + Bool :: b8 RUsage :: struct { @@ -24,3 +26,10 @@ RUsage :: struct { ru_nivcsw: c.long, } +foreign system { + __error :: proc() -> ^i32 --- +} + +errno :: #force_inline proc "contextless" () -> i32 { + return __error()^ +} diff --git a/core/sys/darwin/sync.odin b/core/sys/darwin/sync.odin new file mode 100644 index 000000000..b9fc82ecc --- /dev/null +++ b/core/sys/darwin/sync.odin @@ -0,0 +1,309 @@ +package darwin + +foreign import system "system:System.framework" + +// #define OS_WAIT_ON_ADDR_AVAILABILITY \ +// __API_AVAILABLE(macos(14.4), ios(17.4), tvos(17.4), watchos(10.4)) +when ODIN_OS == .Darwin { + when ODIN_PLATFORM_SUBTARGET == .iOS && MINIMUM_OS_VERSION > 17_04_00 { + WAIT_ON_ADDRESS_AVAILABLE :: true + } else when MINIMUM_OS_VERSION > 14_04_00 { + WAIT_ON_ADDRESS_AVAILABLE :: true + } else { + WAIT_ON_ADDRESS_AVAILABLE :: false + } +} else { + WAIT_ON_ADDRESS_AVAILABLE :: false +} + +os_sync_wait_on_address_flag :: enum u32 { + // This flag should be used as a default flag when no other flags listed below are required. + NONE, + + // This flag should be used when synchronizing among multiple processes by + // placing the @addr passed to os_sync_wait_on_address and its variants + // in a shared memory region. + // + // When using this flag, it is important to pass OS_SYNC_WAKE_BY_ADDRESS_SHARED + // flag along with the exact same @addr to os_sync_wake_by_address_any and + // its variants to correctly find and wake up blocked waiters on the @addr. + // + // This flag should not be used when synchronizing among multiple threads of + // a single process. It allows the kernel to perform performance optimizations + // as the @addr is local to the calling process. + SHARED, +} + +os_sync_wait_on_address_flags :: bit_set[os_sync_wait_on_address_flag; u32] + +os_sync_wake_by_address_flag :: enum u32 { + // This flag should be used as a default flag when no other flags listed below are required. + NONE, + + // This flag should be used when synchronizing among multiple processes by + // placing the @addr passed to os_sync_wake_by_address_any and its variants + // in a shared memory region. + // + // When using this flag, it is important to pass OS_SYNC_WAIT_ON_ADDRESS_SHARED + // flag along with the exact same @addr to os_sync_wait_on_address and + // its variants to correctly find and wake up blocked waiters on the @addr. + // + // This flag should not be used when synchronizing among multiple threads of + // a single process. It allows the kernel to perform performance optimizations + // as the @addr is local the calling process. + SHARED, +} + +os_sync_wake_by_address_flags :: bit_set[os_sync_wake_by_address_flag; u32] + +os_clockid :: enum u32 { + MACH_ABSOLUTE_TIME = 32, +} + +foreign system { + // This function provides an atomic compare-and-wait functionality that + // can be used to implement other higher level synchronization primitives. + // + // It reads a value from @addr, compares it to expected @value and blocks + // the calling thread if they are equal. This sequence of operations is + // done atomically with respect to other concurrent operations that can + // be performed on this @addr by other threads using this same function + // or os_sync_wake_by_addr variants. At this point, the blocked calling + // thread is considered to be a waiter on this @addr, waiting to be woken + // up by a call to os_sync_wake_by_addr variants. If the value at @addr + // turns out to be different than expected, the calling thread returns + // immediately without blocking. + // + // This function is expected to be used for implementing synchronization + // primitives that do not have a sense of ownership (e.g. condition + // variables, semaphores) as it does not provide priority inversion avoidance. + // For locking primitives, it is recommended that you use existing OS + // primitives such as os_unfair_lock API family / pthread mutex or + // std::mutex. + // + // @param addr + // The userspace address to be used for atomic compare-and-wait. + // This address must be aligned to @size. + // + // @param value + // The value expected at @addr. + // + // @param size + // The size of @value, in bytes. This can be either 4 or 8 today. + // For @value of @size 4 bytes, the upper 4 bytes of @value are ignored. + // + // @param flags + // Flags to alter behavior of os_sync_wait_on_address. + // See os_sync_wait_on_address_flags_t. + // + // @return + // If the calling thread is woken up by a call to os_sync_wake_by_addr + // variants or the value at @addr is different than expected, this function + // returns successfully and the return value indicates the number + // of outstanding waiters blocked on this address. + // In the event of an error, returns -1 with errno set to indicate the error. + // + // EINVAL : Invalid flags or size. + // EINVAL : The @addr passed is NULL or misaligned. + // EINVAL : The operation associated with existing kernel state + // at this @addr is inconsistent with what the caller + // has requested. + // It is important to make sure consistent values are + // passed across wait and wake APIs for @addr, @size + // and the shared memory specification + // (See os_sync_wait_on_address_flags_t). + // + // It is possible for the os_sync_wait_on_address and its variants to perform + // an early return in the event of following errors where user may want to + // re-try the wait operation. E.g. low memory conditions could cause such early + // return. + // It is important to read the current value at the @addr before re-trying + // to ensure that the new value still requires waiting on @addr. + // + // ENOMEM : Unable to allocate memory for kernel internal data + // structures. + // EINTR : The syscall was interrupted / spurious wake up. + // EFAULT : Unable to read value from the @addr. Kernel copyin failed. + // It is possible to receive EFAULT error in following cases: + // 1. The @addr is an invalid address. This is a programmer error. + // 2. The @addr is valid; but, this is a transient error such as + // due to low memory conditions. User may want to re-try the wait + // operation. + // Following code snippet illustrates a possible re-try loop. + // + // retry: + // current = atomic_load_explicit(addr, memory_order_relaxed); + // if (current != expected) { + // int ret = os_sync_wait_on_address(addr, current, size, flags); + // if ((ret < 0) && ((errno == EINTR) || (errno == EFAULT))) { + // goto retry; + // } + // } + // + os_sync_wait_on_address :: proc( + addr: rawptr, + value: u64, + size: uint, + flags: os_sync_wait_on_address_flags, + ) -> i32 --- + + // This function is a variant of os_sync_wait_on_address that + // allows the calling thread to specify a deadline + // until which it is willing to block. + // + // @param addr + // The userspace address to be used for atomic compare-and-wait. + // This address must be aligned to @size. + // + // @param value + // The value expected at @addr. + // + // @param size + // The size of @value, in bytes. This can be either 4 or 8 today. + // For @value of @size 4 bytes, the upper 4 bytes of @value are ignored. + // + // @param flags + // Flags to alter behavior of os_sync_wait_on_address_with_deadline. + // See os_sync_wait_on_address_flags_t. + // + // @param clockid + // This value anchors @deadline argument to a specific clock id. + // See os_clockid_t. + // + // @param deadline + // This value is used to specify a deadline until which the calling + // thread is willing to block. + // Passing zero for the @deadline results in an error being returned. + // It is recommended to use os_sync_wait_on_address API to block + // indefinitely until woken up by a call to os_sync_wake_by_address_any + // or os_sync_wake_by_address_all APIs. + // + // @return + // If the calling thread is woken up by a call to os_sync_wake_by_addr + // variants or the value at @addr is different than expected, this function + // returns successfully and the return value indicates the number + // of outstanding waiters blocked on this address. + // In the event of an error, returns -1 with errno set to indicate the error. + // + // In addition to errors returned by os_sync_wait_on_address, this function + // can return the following additional error codes. + // + // EINVAL : Invalid clock id. + // EINVAL : The @deadline passed is 0. + // ETIMEDOUT : Deadline expired. + os_sync_wait_on_address_with_deadline :: proc( + addr: rawptr, + value: u64, + size: uint, + flags: os_sync_wait_on_address_flags, + clockid: os_clockid, + deadline: u64, + ) -> i32 --- + + // This function is a variant of os_sync_wait_on_address that + // allows the calling thread to specify a timeout + // until which it is willing to block. + // + // @param addr + // The userspace address to be used for atomic compare-and-wait. + // This address must be aligned to @size. + // + // @param value + // The value expected at @addr. + // + // @param size + // The size of @value, in bytes. This can be either 4 or 8 today. + // For @value of @size 4 bytes, the upper 4 bytes of @value are ignored. + // + // @param flags + // Flags to alter behavior of os_sync_wait_on_address_with_timeout. + // See os_sync_wait_on_address_flags_t. + // + // @param clockid + // This value anchors @timeout_ns argument to a specific clock id. + // See os_clockid_t. + // + // @param timeout_ns + // This value is used to specify a timeout in nanoseconds until which + // the calling thread is willing to block. + // Passing zero for the @timeout_ns results in an error being returned. + // It is recommended to use os_sync_wait_on_address API to block + // indefinitely until woken up by a call to os_sync_wake_by_address_any + // or os_sync_wake_by_address_all APIs. + // + // @return + // If the calling thread is woken up by a call to os_sync_wake_by_address + // variants or the value at @addr is different than expected, this function + // returns successfully and the return value indicates the number + // of outstanding waiters blocked on this address. + // In the event of an error, returns -1 with errno set to indicate the error. + // + // In addition to errors returned by os_sync_wait_on_address, this function + // can return the following additional error codes. + // + // EINVAL : Invalid clock id. + // EINVAL : The @timeout_ns passed is 0. + // ETIMEDOUT : Timeout expired. + os_sync_wait_on_address_with_timeout :: proc( + addr: rawptr, + value: u64, + size: uint, + flags: os_sync_wait_on_address_flags, + clockid: os_clockid, + timeout_ns: u64, + ) -> i32 --- + + // This function wakes up one waiter out of all those blocked in os_sync_wait_on_address + // or its variants on the @addr. No guarantee is provided about which + // specific waiter is woken up. + // + // @param addr + // The userspace address to be used for waking up the blocked waiter. + // It should be same as what is passed to os_sync_wait_on_address or its variants. + // + // @param size + // The size of lock value, in bytes. This can be either 4 or 8 today. + // It should be same as what is passed to os_sync_wait_on_address or its variants. + // + // @param flags + // Flags to alter behavior of os_sync_wake_by_address_any. + // See os_sync_wake_by_address_flags_t. + // + // @return + // Returns 0 on success. + // In the event of an error, returns -1 with errno set to indicate the error. + // + // EINVAL : Invalid flags or size. + // EINVAL : The @addr passed is NULL. + // EINVAL : The operation associated with existing kernel state + // at this @addr is inconsistent with what caller + // has requested. + // It is important to make sure consistent values are + // passed across wait and wake APIs for @addr, @size + // and the shared memory specification + // (See os_sync_wake_by_address_flags_t). + // ENOENT : No waiter(s) found waiting on the @addr. + os_sync_wake_by_address_any :: proc(addr: rawptr, size: uint, flags: os_sync_wait_on_address_flags) -> i32 --- + + // This function is a variant of os_sync_wake_by_address_any that wakes up all waiters + // blocked in os_sync_wait_on_address or its variants. + // + // @param addr + // The userspace address to be used for waking up the blocked waiters. + // It should be same as what is passed to os_sync_wait_on_address or its variants. + // + // @param size + // The size of lock value, in bytes. This can be either 4 or 8 today. + // It should be same as what is passed to os_sync_wait_on_address or its variants. + // + // @param flags + // Flags to alter behavior of os_sync_wake_by_address_all. + // See os_sync_wake_by_address_flags_t. + // + // @return + // Returns 0 on success. + // In the event of an error, returns -1 with errno set to indicate the error. + // + // This function returns same error codes as returned by os_sync_wait_on_address. + os_sync_wake_by_address_all :: proc(addr: rawptr, size: uint, flags: os_sync_wait_on_address_flags) -> i32 --- +} diff --git a/src/checker.cpp b/src/checker.cpp index 72c0ae574..797cdb5f1 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1097,6 +1097,15 @@ gb_internal void init_universal(void) { scope_insert(intrinsics_pkg->scope, t_atomic_memory_order->Named.type_name); } + { + int minimum_os_version = 0; + if (build_context.minimum_os_version_string != "") { + int major, minor, revision = 0; + sscanf(cast(const char *)(build_context.minimum_os_version_string.text), "%d.%d.%d", &major, &minor, &revision); + minimum_os_version = (major*10000)+(minor*100)+revision; + } + add_global_constant("MINIMUM_OS_VERSION", t_untyped_integer, exact_value_i64(minimum_os_version)); + } add_global_bool_constant("ODIN_DEBUG", bc->ODIN_DEBUG); add_global_bool_constant("ODIN_DISABLE_ASSERT", bc->ODIN_DISABLE_ASSERT); diff --git a/src/threading.cpp b/src/threading.cpp index a469435d2..3197b19a3 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -760,6 +760,11 @@ gb_internal void futex_wait(Futex *f, Footex val) { #elif defined(GB_SYSTEM_OSX) +#if __has_include() + #define DARWIN_WAIT_ON_ADDRESS_AVAILABLE + #include +#endif + #define UL_COMPARE_AND_WAIT 0x00000001 #define ULF_NO_ERRNO 0x01000000 @@ -767,6 +772,23 @@ extern "C" int __ulock_wait(uint32_t operation, void *addr, uint64_t value, uint extern "C" int __ulock_wake(uint32_t operation, void *addr, uint64_t wake_value); gb_internal void futex_signal(Futex *f) { + #ifdef DARWIN_WAIT_ON_ADDRESS_AVAILABLE + if (__builtin_available(macOS 14.4, *)) { + for (;;) { + int ret = os_sync_wake_by_address_any(f, sizeof(Futex), OS_SYNC_WAKE_BY_ADDRESS_NONE); + if (ret >= 0) { + return; + } + if (errno == EINTR || errno == EFAULT) { + continue; + } + if (errno == ENOENT) { + return; + } + GB_PANIC("Failed in futex wake %d %d!\n", ret, errno); + } + } else { + #endif for (;;) { int ret = __ulock_wake(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, 0); if (ret >= 0) { @@ -780,9 +802,29 @@ gb_internal void futex_signal(Futex *f) { } GB_PANIC("Failed in futex wake!\n"); } + #ifdef DARWIN_WAIT_ON_ADDRESS_AVAILABLE + } + #endif } gb_internal void futex_broadcast(Futex *f) { + #ifdef DARWIN_WAIT_ON_ADDRESS_AVAILABLE + if (__builtin_available(macOS 14.4, *)) { + for (;;) { + int ret = os_sync_wake_by_address_all(f, sizeof(Footex), OS_SYNC_WAKE_BY_ADDRESS_NONE); + if (ret >= 0) { + return; + } + if (errno == EINTR || errno == EFAULT) { + continue; + } + if (errno == ENOENT) { + return; + } + GB_PANIC("Failed in futext wake %d %d!\n", ret, errno); + } + } else { + #endif for (;;) { enum { ULF_WAKE_ALL = 0x00000100 }; int ret = __ulock_wake(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO | ULF_WAKE_ALL, f, 0); @@ -797,9 +839,32 @@ gb_internal void futex_broadcast(Futex *f) { } GB_PANIC("Failed in futex wake!\n"); } + #ifdef DARWIN_WAIT_ON_ADDRESS_AVAILABLE + } + #endif } gb_internal void futex_wait(Futex *f, Footex val) { + #ifdef DARWIN_WAIT_ON_ADDRESS_AVAILABLE + if (__builtin_available(macOS 14.4, *)) { + for (;;) { + int ret = os_sync_wait_on_address(f, cast(uint64_t)(val), sizeof(Footex), OS_SYNC_WAIT_ON_ADDRESS_NONE); + if (ret >= 0) { + if (*f != val) { + return; + } + continue; + } + if (errno == EINTR || errno == EFAULT) { + continue; + } + if (errno == ENOENT) { + return; + } + GB_PANIC("Failed in futex wait %d %d!\n", ret, errno); + } + } else { + #endif for (;;) { int ret = __ulock_wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, f, val, 0); if (ret >= 0) { @@ -817,7 +882,11 @@ gb_internal void futex_wait(Futex *f, Footex val) { GB_PANIC("Failed in futex wait!\n"); } + #ifdef DARWIN_WAIT_ON_ADDRESS_AVAILABLE + } + #endif } + #elif defined(GB_SYSTEM_WINDOWS) gb_internal void futex_signal(Futex *f) { -- cgit v1.2.3 From 6d4f30de1a85fe51159808d70a342c1c915d15de Mon Sep 17 00:00:00 2001 From: rick-masters Date: Sun, 24 Mar 2024 16:28:55 +0000 Subject: Fix fields_wait_signal futex. --- src/check_builtin.cpp | 2 ++ src/check_expr.cpp | 1 + src/check_type.cpp | 4 ++++ src/threading.cpp | 2 +- 4 files changed, 8 insertions(+), 1 deletion(-) (limited to 'src/threading.cpp') diff --git a/src/check_builtin.cpp b/src/check_builtin.cpp index 53e4acbd1..f4aa9567d 100644 --- a/src/check_builtin.cpp +++ b/src/check_builtin.cpp @@ -3393,6 +3393,7 @@ gb_internal bool check_builtin_procedure(CheckerContext *c, Operand *operand, As elem->Struct.tags = gb_alloc_array(permanent_allocator(), String, fields.count); elem->Struct.node = dummy_node_struct; type_set_offsets(elem); + wait_signal_set(&elem->Struct.fields_wait_signal); } Type *soa_type = make_soa_struct_slice(c, dummy_node_soa, nullptr, elem); @@ -3766,6 +3767,7 @@ gb_internal bool check_builtin_procedure(CheckerContext *c, Operand *operand, As soa_struct->Struct.tags[i] = old_struct->Struct.tags[i]; } } + wait_signal_set(&soa_struct->Struct.fields_wait_signal); Token token = {}; token.string = str_lit("Base_Type"); diff --git a/src/check_expr.cpp b/src/check_expr.cpp index fd10374c1..d19af4a62 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -8873,6 +8873,7 @@ gb_internal ExprKind check_compound_literal(CheckerContext *c, Operand *o, Ast * break; } + wait_signal_until_available(&t->Struct.fields_wait_signal); isize field_count = t->Struct.fields.count; isize min_field_count = t->Struct.fields.count; for (isize i = min_field_count-1; i >= 0; i--) { diff --git a/src/check_type.cpp b/src/check_type.cpp index 0b9042905..ae79d4edc 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -2490,6 +2490,7 @@ gb_internal Type *get_map_cell_type(Type *type) { s->Struct.fields[0] = alloc_entity_field(scope, make_token_ident("v"), alloc_type_array(type, len), false, 0, EntityState_Resolved); s->Struct.fields[1] = alloc_entity_field(scope, make_token_ident("_"), alloc_type_array(t_u8, padding), false, 1, EntityState_Resolved); s->Struct.scope = scope; + wait_signal_set(&s->Struct.fields_wait_signal); gb_unused(type_size_of(s)); return s; @@ -2520,6 +2521,7 @@ gb_internal void init_map_internal_types(Type *type) { metadata_type->Struct.fields[4] = alloc_entity_field(metadata_scope, make_token_ident("value_cell"), value_cell, false, 4, EntityState_Resolved); metadata_type->Struct.scope = metadata_scope; metadata_type->Struct.node = nullptr; + wait_signal_set(&metadata_type->Struct.fields_wait_signal); gb_unused(type_size_of(metadata_type)); @@ -2537,6 +2539,7 @@ gb_internal void init_map_internal_types(Type *type) { debug_type->Struct.fields[3] = alloc_entity_field(scope, make_token_ident("__metadata"), metadata_type, false, 3, EntityState_Resolved); debug_type->Struct.scope = scope; debug_type->Struct.node = nullptr; + wait_signal_set(&debug_type->Struct.fields_wait_signal); gb_unused(type_size_of(debug_type)); @@ -2832,6 +2835,7 @@ gb_internal Type *make_soa_struct_internal(CheckerContext *ctx, Ast *array_typ_e add_entity(ctx, scope, nullptr, base_type_entity); add_type_info_type(ctx, soa_struct); + wait_signal_set(&soa_struct->Struct.fields_wait_signal); return soa_struct; } diff --git a/src/threading.cpp b/src/threading.cpp index a469435d2..9e4a1607c 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -113,7 +113,7 @@ struct Wait_Signal { gb_internal void wait_signal_until_available(Wait_Signal *ws) { if (ws->futex.load() == 0) { - futex_wait(&ws->futex, 1); + futex_wait(&ws->futex, 0); } } -- cgit v1.2.3 From e5629dafd0be1be42a3bd183a09ff82492b6b386 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 25 Mar 2024 13:23:43 +0000 Subject: Potentially fix a race condition with parapoly types (related to #3328) --- src/check_expr.cpp | 11 +++- src/check_type.cpp | 179 +++++++++++++++++++++++++++-------------------------- src/checker.hpp | 9 ++- src/threading.cpp | 2 - 4 files changed, 104 insertions(+), 97 deletions(-) (limited to 'src/threading.cpp') diff --git a/src/check_expr.cpp b/src/check_expr.cpp index fd10374c1..44d65e376 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -79,7 +79,6 @@ gb_internal Type * check_type_expr (CheckerContext *c, Ast *exp gb_internal Type * make_optional_ok_type (Type *value, bool typed=true); gb_internal Entity * check_selector (CheckerContext *c, Operand *operand, Ast *node, Type *type_hint); gb_internal Entity * check_ident (CheckerContext *c, Operand *o, Ast *n, Type *named_type, Type *type_hint, bool allow_import_name); -gb_internal Entity * find_polymorphic_record_entity (CheckerContext *c, Type *original_type, isize param_count, Array const &ordered_operands, bool *failure); gb_internal void check_not_tuple (CheckerContext *c, Operand *operand); gb_internal void convert_to_typed (CheckerContext *c, Operand *operand, Type *target_type); gb_internal gbString expr_to_string (Ast *expression); @@ -121,6 +120,8 @@ gb_internal isize get_procedure_param_count_excluding_defaults(Type *pt, isize * gb_internal bool is_expr_inferred_fixed_array(Ast *type_expr); +gb_internal Entity *find_polymorphic_record_entity(GenTypesData *found_gen_types, isize param_count, Array const &ordered_operands); + enum LoadDirectiveResult { LoadDirective_Success = 0, LoadDirective_Error = 1, @@ -7171,8 +7172,12 @@ gb_internal CallArgumentError check_polymorphic_record_type(CheckerContext *c, O } { - bool failure = false; - Entity *found_entity = find_polymorphic_record_entity(c, original_type, param_count, ordered_operands, &failure); + GenTypesData *found_gen_types = ensure_polymorphic_record_entity_has_gen_types(c, original_type); + + mutex_lock(&found_gen_types->mutex); + defer (mutex_unlock(&found_gen_types->mutex)); + Entity *found_entity = find_polymorphic_record_entity(found_gen_types, param_count, ordered_operands); + if (found_entity) { operand->mode = Addressing_Type; operand->type = found_entity->type; diff --git a/src/check_type.cpp b/src/check_type.cpp index 0b9042905..e22d4b62b 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -260,84 +260,21 @@ gb_internal bool check_custom_align(CheckerContext *ctx, Ast *node, i64 *align_, } -gb_internal Entity *find_polymorphic_record_entity(CheckerContext *ctx, Type *original_type, isize param_count, Array const &ordered_operands, bool *failure) { - rw_mutex_shared_lock(&ctx->info->gen_types_mutex); // @@global - - auto *found_gen_types = map_get(&ctx->info->gen_types, original_type); - if (found_gen_types == nullptr) { - rw_mutex_shared_unlock(&ctx->info->gen_types_mutex); // @@global - return nullptr; - } - - rw_mutex_shared_lock(&found_gen_types->mutex); // @@local - defer (rw_mutex_shared_unlock(&found_gen_types->mutex)); // @@local - - rw_mutex_shared_unlock(&ctx->info->gen_types_mutex); // @@global - - for (Entity *e : found_gen_types->types) { - Type *t = base_type(e->type); - TypeTuple *tuple = nullptr; - switch (t->kind) { - case Type_Struct: - if (t->Struct.polymorphic_params) { - tuple = &t->Struct.polymorphic_params->Tuple; - } - break; - case Type_Union: - if (t->Union.polymorphic_params) { - tuple = &t->Union.polymorphic_params->Tuple; - } - break; - } - GB_ASSERT_MSG(tuple != nullptr, "%s :: %s", type_to_string(e->type), type_to_string(t)); - GB_ASSERT(param_count == tuple->variables.count); - - bool skip = false; - - for (isize j = 0; j < param_count; j++) { - Entity *p = tuple->variables[j]; - Operand o = {}; - if (j < ordered_operands.count) { - o = ordered_operands[j]; - } - if (o.expr == nullptr) { - continue; - } - Entity *oe = entity_of_node(o.expr); - if (p == oe) { - // NOTE(bill): This is the same type, make sure that it will be be same thing and use that - // Saves on a lot of checking too below - continue; - } - - if (p->kind == Entity_TypeName) { - if (is_type_polymorphic(o.type)) { - // NOTE(bill): Do not add polymorphic version to the gen_types - skip = true; - break; - } - if (!are_types_identical(o.type, p->type)) { - skip = true; - break; - } - } else if (p->kind == Entity_Constant) { - if (!compare_exact_values(Token_CmpEq, o.value, p->Constant.value)) { - skip = true; - break; - } - if (!are_types_identical(o.type, p->type)) { - skip = true; - break; - } - } else { - GB_PANIC("Unknown entity kind"); - } - } - if (!skip) { - return e; - } +gb_internal GenTypesData *ensure_polymorphic_record_entity_has_gen_types(CheckerContext *ctx, Type *original_type) { + mutex_lock(&ctx->info->gen_types_mutex); // @@global + + GenTypesData *found_gen_types = nullptr; + auto *found_gen_types_ptr = map_get(&ctx->info->gen_types, original_type); + if (found_gen_types_ptr == nullptr) { + GenTypesData *gen_types = gb_alloc_item(permanent_allocator(), GenTypesData); + gen_types->types = array_make(heap_allocator()); + map_set(&ctx->info->gen_types, original_type, gen_types); + found_gen_types_ptr = map_get(&ctx->info->gen_types, original_type); } - return nullptr; + found_gen_types = *found_gen_types_ptr; + GB_ASSERT(found_gen_types != nullptr); + mutex_unlock(&ctx->info->gen_types_mutex); // @@global + return found_gen_types; } @@ -367,19 +304,16 @@ gb_internal void add_polymorphic_record_entity(CheckerContext *ctx, Ast *node, T // TODO(bill): Is this even correct? Or should the metadata be copied? e->TypeName.objc_metadata = original_type->Named.type_name->TypeName.objc_metadata; - rw_mutex_lock(&ctx->info->gen_types_mutex); - auto *found_gen_types = map_get(&ctx->info->gen_types, original_type); - if (found_gen_types) { - rw_mutex_lock(&found_gen_types->mutex); - array_add(&found_gen_types->types, e); - rw_mutex_unlock(&found_gen_types->mutex); - } else { - GenTypesData gen_types = {}; - gen_types.types = array_make(heap_allocator()); - array_add(&gen_types.types, e); - map_set(&ctx->info->gen_types, original_type, gen_types); + auto *found_gen_types = ensure_polymorphic_record_entity_has_gen_types(ctx, original_type); + mutex_lock(&found_gen_types->mutex); + defer (mutex_unlock(&found_gen_types->mutex)); + + for (Entity *prev : found_gen_types->types) { + if (prev == e) { + return; + } } - rw_mutex_unlock(&ctx->info->gen_types_mutex); + array_add(&found_gen_types->types, e); } @@ -612,6 +546,73 @@ gb_internal bool check_record_poly_operand_specialization(CheckerContext *ctx, T return true; } +gb_internal Entity *find_polymorphic_record_entity(GenTypesData *found_gen_types, isize param_count, Array const &ordered_operands) { + for (Entity *e : found_gen_types->types) { + Type *t = base_type(e->type); + TypeTuple *tuple = nullptr; + switch (t->kind) { + case Type_Struct: + if (t->Struct.polymorphic_params) { + tuple = &t->Struct.polymorphic_params->Tuple; + } + break; + case Type_Union: + if (t->Union.polymorphic_params) { + tuple = &t->Union.polymorphic_params->Tuple; + } + break; + } + GB_ASSERT_MSG(tuple != nullptr, "%s :: %s", type_to_string(e->type), type_to_string(t)); + GB_ASSERT(param_count == tuple->variables.count); + + bool skip = false; + + for (isize j = 0; j < param_count; j++) { + Entity *p = tuple->variables[j]; + Operand o = {}; + if (j < ordered_operands.count) { + o = ordered_operands[j]; + } + if (o.expr == nullptr) { + continue; + } + Entity *oe = entity_of_node(o.expr); + if (p == oe) { + // NOTE(bill): This is the same type, make sure that it will be be same thing and use that + // Saves on a lot of checking too below + continue; + } + + if (p->kind == Entity_TypeName) { + if (is_type_polymorphic(o.type)) { + // NOTE(bill): Do not add polymorphic version to the gen_types + skip = true; + break; + } + if (!are_types_identical(o.type, p->type)) { + skip = true; + break; + } + } else if (p->kind == Entity_Constant) { + if (!compare_exact_values(Token_CmpEq, o.value, p->Constant.value)) { + skip = true; + break; + } + if (!are_types_identical(o.type, p->type)) { + skip = true; + break; + } + } else { + GB_PANIC("Unknown entity kind"); + } + } + if (!skip) { + return e; + } + } + return nullptr; +}; + gb_internal void check_struct_type(CheckerContext *ctx, Type *struct_type, Ast *node, Array *poly_operands, Type *named_type, Type *original_type_for_poly) { GB_ASSERT(is_type_struct(struct_type)); diff --git a/src/checker.hpp b/src/checker.hpp index eea99578e..e0dc54a87 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -360,7 +360,7 @@ struct GenProcsData { struct GenTypesData { Array types; - RwMutex mutex; + RecursiveMutex mutex; }; // CheckerInfo stores all the symbol information for a type-checked program @@ -400,8 +400,8 @@ struct CheckerInfo { RecursiveMutex lazy_mutex; // Mutex required for lazy type checking of specific files - RwMutex gen_types_mutex; - PtrMap gen_types; + BlockingMutex gen_types_mutex; + PtrMap gen_types; BlockingMutex type_info_mutex; // NOT recursive Array type_info_types; @@ -560,3 +560,6 @@ gb_internal void init_core_context(Checker *c); gb_internal void init_mem_allocator(Checker *c); gb_internal void add_untyped_expressions(CheckerInfo *cinfo, UntypedExprInfoMap *untyped); + + +gb_internal GenTypesData *ensure_polymorphic_record_entity_has_gen_types(CheckerContext *ctx, Type *original_type); \ No newline at end of file diff --git a/src/threading.cpp b/src/threading.cpp index a469435d2..d9538f66e 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -122,8 +122,6 @@ gb_internal void wait_signal_set(Wait_Signal *ws) { futex_broadcast(&ws->futex); } - - struct MutexGuard { MutexGuard() = delete; MutexGuard(MutexGuard const &) = delete; -- cgit v1.2.3