From dcf4e51787119d8dcf86ca0195d3a07aea6d47a8 Mon Sep 17 00:00:00 2001 From: hikari Date: Wed, 7 Jun 2023 19:11:16 +0300 Subject: [PATCH 1/3] [core:thread] Added `self_cleanup` flag to properly auto-clean threads --- core/thread/thread.odin | 128 ++++-------------- core/thread/thread_unix.odin | 5 + core/thread/thread_windows.odin | 6 + tests/documentation/documentation_tester.odin | 4 +- 4 files changed, 41 insertions(+), 102 deletions(-) diff --git a/core/thread/thread.odin b/core/thread/thread.odin index fe502c5ae..c84317055 100644 --- a/core/thread/thread.odin +++ b/core/thread/thread.odin @@ -38,7 +38,7 @@ Thread :: struct { IMPORTANT: By default, the thread proc will get the same context as `main()` gets. - In this sitation, the thread will get a new temporary allocator which will be cleaned up when the thread dies. + In this situation, the thread will get a new temporary allocator which will be cleaned up when the thread dies. ***This does NOT happen when you set `init_context`.*** This means that if you set `init_context`, but still have the `temp_allocator` field set to the default temp allocator, then you'll need to call `runtime.default_temp_allocator_destroy(auto_cast the_thread.init_context.temp_allocator.data)` manually, @@ -47,6 +47,8 @@ Thread :: struct { */ init_context: Maybe(runtime.Context), + // Indicates whether the thread will free itself when it completes + self_cleanup: bool, creation_allocator: mem.Allocator, } @@ -101,125 +103,46 @@ yield :: proc() { run :: proc(fn: proc(), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) { - thread_proc :: proc(t: ^Thread) { - fn := cast(proc())t.data - fn() - destroy(t) - } - t := create(thread_proc, priority) - t.data = rawptr(fn) - t.init_context = init_context - start(t) + create_and_start(fn, init_context, priority, true) } run_with_data :: proc(data: rawptr, fn: proc(data: rawptr), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) { - thread_proc :: proc(t: ^Thread) { - fn := cast(proc(rawptr))t.data - assert(t.user_index >= 1) - data := t.user_args[0] - fn(data) - destroy(t) - } - t := create(thread_proc, priority) - t.data = rawptr(fn) - t.user_index = 1 - t.user_args = data - t.init_context = init_context - start(t) + create_and_start_with_data(data, fn, init_context, priority, true) } run_with_poly_data :: proc(data: $T, fn: proc(data: T), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) where size_of(T) <= size_of(rawptr) { - thread_proc :: proc(t: ^Thread) { - fn := cast(proc(T))t.data - assert(t.user_index >= 1) - data := (^T)(&t.user_args[0])^ - fn(data) - destroy(t) - } - t := create(thread_proc, priority) - t.data = rawptr(fn) - t.user_index = 1 - data := data - mem.copy(&t.user_args[0], &data, size_of(data)) - t.init_context = init_context - start(t) + create_and_start_with_poly_data(data, fn, init_context, priority, true) } run_with_poly_data2 :: proc(arg1: $T1, arg2: $T2, fn: proc(T1, T2), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) where size_of(T1) <= size_of(rawptr), size_of(T2) <= size_of(rawptr) { - thread_proc :: proc(t: ^Thread) { - fn := cast(proc(T1, T2))t.data - assert(t.user_index >= 2) - arg1 := (^T1)(&t.user_args[0])^ - arg2 := (^T2)(&t.user_args[1])^ - fn(arg1, arg2) - destroy(t) - } - t := create(thread_proc, priority) - t.data = rawptr(fn) - t.user_index = 2 - arg1, arg2 := arg1, arg2 - mem.copy(&t.user_args[0], &arg1, size_of(arg1)) - mem.copy(&t.user_args[1], &arg2, size_of(arg2)) - t.init_context = init_context - start(t) + create_and_start_with_poly_data2(arg1, arg2, fn, init_context, priority, true) } run_with_poly_data3 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, fn: proc(arg1: T1, arg2: T2, arg3: T3), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) where size_of(T1) <= size_of(rawptr), size_of(T2) <= size_of(rawptr), size_of(T3) <= size_of(rawptr) { - thread_proc :: proc(t: ^Thread) { - fn := cast(proc(T1, T2, T3))t.data - assert(t.user_index >= 3) - arg1 := (^T1)(&t.user_args[0])^ - arg2 := (^T2)(&t.user_args[1])^ - arg3 := (^T3)(&t.user_args[2])^ - fn(arg1, arg2, arg3) - destroy(t) - } - t := create(thread_proc, priority) - t.data = rawptr(fn) - t.user_index = 3 - arg1, arg2, arg3 := arg1, arg2, arg3 - mem.copy(&t.user_args[0], &arg1, size_of(arg1)) - mem.copy(&t.user_args[1], &arg2, size_of(arg2)) - mem.copy(&t.user_args[2], &arg3, size_of(arg3)) - t.init_context = init_context - start(t) + create_and_start_with_poly_data3(arg1, arg2, arg3, fn, init_context, priority, true) } run_with_poly_data4 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, arg4: $T4, fn: proc(arg1: T1, arg2: T2, arg3: T3, arg4: T4), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) where size_of(T1) <= size_of(rawptr), size_of(T2) <= size_of(rawptr), size_of(T3) <= size_of(rawptr) { - thread_proc :: proc(t: ^Thread) { - fn := cast(proc(T1, T2, T3, T4))t.data - assert(t.user_index >= 4) - arg1 := (^T1)(&t.user_args[0])^ - arg2 := (^T2)(&t.user_args[1])^ - arg3 := (^T3)(&t.user_args[2])^ - arg4 := (^T4)(&t.user_args[3])^ - fn(arg1, arg2, arg3, arg4) - destroy(t) - } - t := create(thread_proc, priority) - t.data = rawptr(fn) - t.user_index = 4 - arg1, arg2, arg3, arg4 := arg1, arg2, arg3, arg4 - mem.copy(&t.user_args[0], &arg1, size_of(arg1)) - mem.copy(&t.user_args[1], &arg2, size_of(arg2)) - mem.copy(&t.user_args[2], &arg3, size_of(arg3)) - mem.copy(&t.user_args[3], &arg4, size_of(arg4)) - t.init_context = init_context - start(t) + create_and_start_with_poly_data4(arg1, arg2, arg3, arg4, fn, init_context, priority, true) } - -create_and_start :: proc(fn: Thread_Proc, init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) -> ^Thread { - t := create(fn, priority) +create_and_start :: proc(fn: proc(), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal, self_cleanup := false) -> ^Thread { + thread_proc :: proc(t: ^Thread) { + fn := cast(proc())t.data + fn() + } + t := create(thread_proc, priority) + t.data = rawptr(fn) + t.self_cleanup = self_cleanup t.init_context = init_context start(t) return t @@ -228,7 +151,7 @@ create_and_start :: proc(fn: Thread_Proc, init_context: Maybe(runtime.Context) = -create_and_start_with_data :: proc(data: rawptr, fn: proc(data: rawptr), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) -> ^Thread { +create_and_start_with_data :: proc(data: rawptr, fn: proc(data: rawptr), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal, self_cleanup := false) -> ^Thread { thread_proc :: proc(t: ^Thread) { fn := cast(proc(rawptr))t.data assert(t.user_index >= 1) @@ -239,12 +162,13 @@ create_and_start_with_data :: proc(data: rawptr, fn: proc(data: rawptr), init_co t.data = rawptr(fn) t.user_index = 1 t.user_args = data + t.self_cleanup = self_cleanup t.init_context = init_context start(t) return t } -create_and_start_with_poly_data :: proc(data: $T, fn: proc(data: T), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) -> ^Thread +create_and_start_with_poly_data :: proc(data: $T, fn: proc(data: T), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal, self_cleanup := false) -> ^Thread where size_of(T) <= size_of(rawptr) { thread_proc :: proc(t: ^Thread) { fn := cast(proc(T))t.data @@ -257,12 +181,13 @@ create_and_start_with_poly_data :: proc(data: $T, fn: proc(data: T), init_contex t.user_index = 1 data := data mem.copy(&t.user_args[0], &data, size_of(data)) + t.self_cleanup = self_cleanup t.init_context = init_context start(t) return t } -create_and_start_with_poly_data2 :: proc(arg1: $T1, arg2: $T2, fn: proc(T1, T2), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) -> ^Thread +create_and_start_with_poly_data2 :: proc(arg1: $T1, arg2: $T2, fn: proc(T1, T2), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal, self_cleanup := false) -> ^Thread where size_of(T1) <= size_of(rawptr), size_of(T2) <= size_of(rawptr) { thread_proc :: proc(t: ^Thread) { @@ -278,12 +203,13 @@ create_and_start_with_poly_data2 :: proc(arg1: $T1, arg2: $T2, fn: proc(T1, T2), arg1, arg2 := arg1, arg2 mem.copy(&t.user_args[0], &arg1, size_of(arg1)) mem.copy(&t.user_args[1], &arg2, size_of(arg2)) + t.self_cleanup = self_cleanup t.init_context = init_context start(t) return t } -create_and_start_with_poly_data3 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, fn: proc(arg1: T1, arg2: T2, arg3: T3), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) -> ^Thread +create_and_start_with_poly_data3 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, fn: proc(arg1: T1, arg2: T2, arg3: T3), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal, self_cleanup := false) -> ^Thread where size_of(T1) <= size_of(rawptr), size_of(T2) <= size_of(rawptr), size_of(T3) <= size_of(rawptr) { @@ -302,11 +228,12 @@ create_and_start_with_poly_data3 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, fn: pr mem.copy(&t.user_args[0], &arg1, size_of(arg1)) mem.copy(&t.user_args[1], &arg2, size_of(arg2)) mem.copy(&t.user_args[2], &arg3, size_of(arg3)) + t.self_cleanup = self_cleanup t.init_context = init_context start(t) return t } -create_and_start_with_poly_data4 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, arg4: $T4, fn: proc(arg1: T1, arg2: T2, arg3: T3, arg4: T4), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal) -> ^Thread +create_and_start_with_poly_data4 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, arg4: $T4, fn: proc(arg1: T1, arg2: T2, arg3: T3, arg4: T4), init_context: Maybe(runtime.Context) = nil, priority := Thread_Priority.Normal, self_cleanup := false) -> ^Thread where size_of(T1) <= size_of(rawptr), size_of(T2) <= size_of(rawptr), size_of(T3) <= size_of(rawptr) { @@ -327,6 +254,7 @@ create_and_start_with_poly_data4 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, arg4: mem.copy(&t.user_args[1], &arg2, size_of(arg2)) mem.copy(&t.user_args[2], &arg3, size_of(arg3)) mem.copy(&t.user_args[3], &arg4, size_of(arg4)) + t.self_cleanup = self_cleanup t.init_context = init_context start(t) return t @@ -360,4 +288,4 @@ _maybe_destroy_default_temp_allocator :: proc(init_context: Maybe(runtime.Contex if context.temp_allocator.procedure == runtime.default_temp_allocator_proc { runtime.default_temp_allocator_destroy(auto_cast context.temp_allocator.data) } -} \ No newline at end of file +} diff --git a/core/thread/thread_unix.odin b/core/thread/thread_unix.odin index 45d2bca2e..eac971dd7 100644 --- a/core/thread/thread_unix.odin +++ b/core/thread/thread_unix.odin @@ -67,6 +67,11 @@ _create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread { sync.unlock(&t.mutex) + if t.self_cleanup { + t.unix_thread = {} + free(t, t.creation_allocator) + } + return nil } diff --git a/core/thread/thread_windows.odin b/core/thread/thread_windows.odin index e62071a1f..eace9a926 100644 --- a/core/thread/thread_windows.odin +++ b/core/thread/thread_windows.odin @@ -47,6 +47,12 @@ _create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread { intrinsics.atomic_store(&t.flags, t.flags + {.Done}) + if t.self_cleanup { + win32.CloseHandle(t.win32_thread) + t.win32_thread = win32.INVALID_HANDLE + free(t, t.creation_allocator) + } + return 0 } diff --git a/tests/documentation/documentation_tester.odin b/tests/documentation/documentation_tester.odin index 086060000..a4d18d1eb 100644 --- a/tests/documentation/documentation_tester.odin +++ b/tests/documentation/documentation_tester.odin @@ -290,7 +290,7 @@ _bad_test_found: bool @(private="file") _spawn_pipe_reader :: proc() { - thread.create_and_start(proc(^thread.Thread) { + thread.run(proc() { stream := os.stream_from_handle(_read_pipe) reader := io.to_reader(stream) sync.post(&_pipe_reader_semaphore) // notify thread is ready @@ -467,4 +467,4 @@ main :: proc() { run_test_suite :: proc() -> bool { return libc.system(fmt.caprintf("%v run verify", g_path_to_odin)) == 0 -} \ No newline at end of file +} From 7b62b81ebd731a03db919f1df24aee4d37bb2c1d Mon Sep 17 00:00:00 2001 From: hikari Date: Wed, 7 Jun 2023 20:03:19 +0300 Subject: [PATCH 2/3] [core:thread] Fix compilation --- core/thread/thread_unix.odin | 2 ++ core/thread/thread_windows.odin | 2 ++ 2 files changed, 4 insertions(+) diff --git a/core/thread/thread_unix.odin b/core/thread/thread_unix.odin index eac971dd7..64f2816e0 100644 --- a/core/thread/thread_unix.odin +++ b/core/thread/thread_unix.odin @@ -69,6 +69,8 @@ _create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread { if t.self_cleanup { t.unix_thread = {} + // NOTE(ftphikari): It doesn't matter which context 'free' received, right? + context = {} free(t, t.creation_allocator) } diff --git a/core/thread/thread_windows.odin b/core/thread/thread_windows.odin index eace9a926..7a7c8b4a2 100644 --- a/core/thread/thread_windows.odin +++ b/core/thread/thread_windows.odin @@ -50,6 +50,8 @@ _create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread { if t.self_cleanup { win32.CloseHandle(t.win32_thread) t.win32_thread = win32.INVALID_HANDLE + // NOTE(ftphikari): It doesn't matter which context 'free' received, right? + context = {} free(t, t.creation_allocator) } From 3b8515beb0aebc08043d040aa01712f37ef51aac Mon Sep 17 00:00:00 2001 From: hikari Date: Wed, 7 Jun 2023 20:52:41 +0300 Subject: [PATCH 3/3] [core:thread] Seeing if this fixes network tests --- core/thread/thread.odin | 23 ++++++++++++++--------- core/thread/thread_js.odin | 10 +--------- core/thread/thread_unix.odin | 9 +-------- core/thread/thread_windows.odin | 9 +-------- 4 files changed, 17 insertions(+), 34 deletions(-) diff --git a/core/thread/thread.odin b/core/thread/thread.odin index c84317055..fd8e59a5d 100644 --- a/core/thread/thread.odin +++ b/core/thread/thread.odin @@ -10,8 +10,16 @@ Thread_Proc :: #type proc(^Thread) MAX_USER_ARGUMENTS :: 8 +Thread_State :: enum u8 { + Started, + Joined, + Done, + Self_Cleanup, +} + Thread :: struct { using specific: Thread_Os_Specific, + flags: bit_set[Thread_State; u8], id: int, procedure: Thread_Proc, @@ -47,9 +55,6 @@ Thread :: struct { */ init_context: Maybe(runtime.Context), - // Indicates whether the thread will free itself when it completes - self_cleanup: bool, - creation_allocator: mem.Allocator, } @@ -142,7 +147,7 @@ create_and_start :: proc(fn: proc(), init_context: Maybe(runtime.Context) = nil, } t := create(thread_proc, priority) t.data = rawptr(fn) - t.self_cleanup = self_cleanup + if self_cleanup do t.flags += {.Self_Cleanup} t.init_context = init_context start(t) return t @@ -162,7 +167,7 @@ create_and_start_with_data :: proc(data: rawptr, fn: proc(data: rawptr), init_co t.data = rawptr(fn) t.user_index = 1 t.user_args = data - t.self_cleanup = self_cleanup + if self_cleanup do t.flags += {.Self_Cleanup} t.init_context = init_context start(t) return t @@ -181,7 +186,7 @@ create_and_start_with_poly_data :: proc(data: $T, fn: proc(data: T), init_contex t.user_index = 1 data := data mem.copy(&t.user_args[0], &data, size_of(data)) - t.self_cleanup = self_cleanup + if self_cleanup do t.flags += {.Self_Cleanup} t.init_context = init_context start(t) return t @@ -203,7 +208,7 @@ create_and_start_with_poly_data2 :: proc(arg1: $T1, arg2: $T2, fn: proc(T1, T2), arg1, arg2 := arg1, arg2 mem.copy(&t.user_args[0], &arg1, size_of(arg1)) mem.copy(&t.user_args[1], &arg2, size_of(arg2)) - t.self_cleanup = self_cleanup + if self_cleanup do t.flags += {.Self_Cleanup} t.init_context = init_context start(t) return t @@ -228,7 +233,7 @@ create_and_start_with_poly_data3 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, fn: pr mem.copy(&t.user_args[0], &arg1, size_of(arg1)) mem.copy(&t.user_args[1], &arg2, size_of(arg2)) mem.copy(&t.user_args[2], &arg3, size_of(arg3)) - t.self_cleanup = self_cleanup + if self_cleanup do t.flags += {.Self_Cleanup} t.init_context = init_context start(t) return t @@ -254,7 +259,7 @@ create_and_start_with_poly_data4 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, arg4: mem.copy(&t.user_args[1], &arg2, size_of(arg2)) mem.copy(&t.user_args[2], &arg3, size_of(arg3)) mem.copy(&t.user_args[3], &arg4, size_of(arg4)) - t.self_cleanup = self_cleanup + if self_cleanup do t.flags += {.Self_Cleanup} t.init_context = init_context start(t) return t diff --git a/core/thread/thread_js.odin b/core/thread/thread_js.odin index 5821ab238..3c4935495 100644 --- a/core/thread/thread_js.odin +++ b/core/thread/thread_js.odin @@ -5,15 +5,7 @@ import "core:intrinsics" import "core:sync" import "core:mem" -Thread_State :: enum u8 { - Started, - Joined, - Done, -} - -Thread_Os_Specific :: struct { - flags: bit_set[Thread_State; u8], -} +Thread_Os_Specific :: struct {} _thread_priority_map := [Thread_Priority]i32{ .Normal = 0, diff --git a/core/thread/thread_unix.odin b/core/thread/thread_unix.odin index 64f2816e0..6e734a03a 100644 --- a/core/thread/thread_unix.odin +++ b/core/thread/thread_unix.odin @@ -8,19 +8,12 @@ import "core:sys/unix" CAS :: intrinsics.atomic_compare_exchange_strong -Thread_State :: enum u8 { - Started, - Joined, - Done, -} - // NOTE(tetra): Aligned here because of core/unix/pthread_linux.odin/pthread_t. // Also see core/sys/darwin/mach_darwin.odin/semaphore_t. Thread_Os_Specific :: struct #align 16 { unix_thread: unix.pthread_t, // NOTE: very large on Darwin, small on Linux. cond: sync.Cond, mutex: sync.Mutex, - flags: bit_set[Thread_State; u8], } // // Creates a thread which will run the given procedure. @@ -67,7 +60,7 @@ _create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread { sync.unlock(&t.mutex) - if t.self_cleanup { + if .Self_Cleanup in t.flags { t.unix_thread = {} // NOTE(ftphikari): It doesn't matter which context 'free' received, right? context = {} diff --git a/core/thread/thread_windows.odin b/core/thread/thread_windows.odin index 7a7c8b4a2..0d004c8c3 100644 --- a/core/thread/thread_windows.odin +++ b/core/thread/thread_windows.odin @@ -6,17 +6,10 @@ import "core:intrinsics" import "core:sync" import win32 "core:sys/windows" -Thread_State :: enum u8 { - Started, - Joined, - Done, -} - Thread_Os_Specific :: struct { win32_thread: win32.HANDLE, win32_thread_id: win32.DWORD, mutex: sync.Mutex, - flags: bit_set[Thread_State; u8], } _thread_priority_map := [Thread_Priority]i32{ @@ -47,7 +40,7 @@ _create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread { intrinsics.atomic_store(&t.flags, t.flags + {.Done}) - if t.self_cleanup { + if .Self_Cleanup in t.flags { win32.CloseHandle(t.win32_thread) t.win32_thread = win32.INVALID_HANDLE // NOTE(ftphikari): It doesn't matter which context 'free' received, right?