Merge pull request #2577 from Tetralux/refactor-init-context

[thread] Refactor handling of 'init_context' + add doc comments for it

core/thread/thread.odin

@@ -14,10 +14,37 @@ Thread :: struct {
 	using specific: Thread_Os_Specific,
 	id:             int,
 	procedure:      Thread_Proc,
+
+	/*
+		These are values that the user can set as they wish, after the thread has been created.
+		This data is easily available to the thread proc.
+
+		These fields can be assigned to directly.
+
+		Should be set after the thread is created, but before it is started.
+	*/
 	data:           rawptr,
 	user_index:     int,
 	user_args:      [MAX_USER_ARGUMENTS]rawptr,
 
+	/*
+		The context to be used as 'context' in the thread proc.
+
+		This field can be assigned to directly, after the thread has been created, but __before__ the thread has been started.
+		This field must not be changed after the thread has started.
+
+		NOTE: If you __don't__ set this, the temp allocator will be managed for you;
+		      If you __do__ set this, then you're expected to handle whatever allocators you set, yourself.
+
+		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.
+		***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,
+		in order to prevent any memory leaks.
+		This call ***must*** be done ***in the thread proc*** because the default temporary allocator uses thread local state!
+	*/
 	init_context: Maybe(runtime.Context),
 
 
@@ -32,6 +59,12 @@ Thread_Priority :: enum {
 	High,
 }
 
+/*
+	Creates a thread in a suspended state with the given priority.
+	To start the thread, call `thread.start()`.
+
+	See `thread.create_and_start()`.
+*/
 create :: proc(procedure: Thread_Proc, priority := Thread_Priority.Normal) -> ^Thread {
 	return _create(procedure, priority)
 }
@@ -298,3 +331,33 @@ create_and_start_with_poly_data4 :: proc(arg1: $T1, arg2: $T2, arg3: $T3, arg4:
 	start(t)
 	return t
 }
+
+
+_select_context_for_thread :: proc(init_context: Maybe(runtime.Context)) -> runtime.Context {
+	ctx, ok := init_context.?
+	if !ok {
+		return runtime.default_context()
+	}
+
+	/*
+		NOTE(tetra, 2023-05-31):
+			Ensure that the temp allocator is thread-safe when the user provides a specific initial context to use.
+			Without this, the thread will use the same temp allocator state as the parent thread, and thus, bork it up.
+	*/
+	if ctx.temp_allocator.procedure == runtime.default_temp_allocator_proc {
+		ctx.temp_allocator.data = &runtime.global_default_temp_allocator_data
+	}
+	return ctx
+}
+
+_maybe_destroy_default_temp_allocator :: proc(init_context: Maybe(runtime.Context)) {
+	if init_context != nil {
+		// NOTE(tetra, 2023-05-31): If the user specifies a custom context for the thread,
+		// then it's entirely up to them to handle whatever allocators they're using.
+		return
+	}
+
+	if context.temp_allocator.procedure == runtime.default_temp_allocator_proc {
+		runtime.default_temp_allocator_destroy(auto_cast context.temp_allocator.data)
+	}
+}

core/thread/thread_unix.odin

@@ -2,7 +2,6 @@
 // +private
 package thread
 
-import "core:runtime"
 import "core:intrinsics"
 import "core:sync"
 import "core:sys/unix"
@@ -27,7 +26,7 @@ Thread_Os_Specific :: struct #align 16 {
 // Creates a thread which will run the given procedure.
 // It then waits for `start` to be called.
 //
-_create :: proc(procedure: Thread_Proc, priority := Thread_Priority.Normal) -> ^Thread {
+_create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread {
 	__linux_thread_entry_proc :: proc "c" (t: rawptr) -> rawptr {
 		t := (^Thread)(t)
 
@@ -36,8 +35,6 @@ _create :: proc(procedure: Thread_Proc, priority := Thread_Priority.Normal) -> ^
 			can_set_thread_cancel_state := unix.pthread_setcancelstate(unix.PTHREAD_CANCEL_DISABLE, nil) == 0
 		}
 
-		context = runtime.default_context()
-
 		sync.lock(&t.mutex)
 
 		t.id = sync.current_thread_id()
@@ -46,9 +43,6 @@ _create :: proc(procedure: Thread_Proc, priority := Thread_Priority.Normal) -> ^
 			sync.wait(&t.cond, &t.mutex)
 		}
 
-		init_context := t.init_context
-		context =	init_context.? or_else runtime.default_context()
-
 		when ODIN_OS != .Darwin {
 			// Enable thread's cancelability.
 			if can_set_thread_cancel_state {
@@ -57,16 +51,22 @@ _create :: proc(procedure: Thread_Proc, priority := Thread_Priority.Normal) -> ^
 			}
 		}
 
-		t.procedure(t)
+		{
+			init_context := t.init_context
+
+			// NOTE(tetra, 2023-05-31): Must do this AFTER thread.start() is called, so that the user can set the init_context, etc!
+			// Here on Unix, we start the OS thread in a running state, and so we manually have it wait on a condition
+			// variable above. We must perform that waiting BEFORE we select the context!
+			context = _select_context_for_thread(init_context)
+			defer _maybe_destroy_default_temp_allocator(init_context)
+
+			t.procedure(t)
+		}
 
 		intrinsics.atomic_store(&t.flags, t.flags + { .Done })
 
 		sync.unlock(&t.mutex)
 
-		if init_context == nil && context.temp_allocator.data == &runtime.global_default_temp_allocator_data {
-			runtime.default_temp_allocator_destroy(auto_cast context.temp_allocator.data)
-		}
-
 		return nil
 	}
 

core/thread/thread_windows.odin

@@ -2,7 +2,6 @@
 //+private
 package thread
 
-import "core:runtime"
 import "core:intrinsics"
 import "core:sync"
 import win32 "core:sys/windows"
@@ -26,24 +25,28 @@ _thread_priority_map := [Thread_Priority]i32{
 	.High = +2,
 }
 
-_create :: proc(procedure: Thread_Proc, priority := Thread_Priority.Normal) -> ^Thread {
+_create :: proc(procedure: Thread_Proc, priority: Thread_Priority) -> ^Thread {
 	win32_thread_id: win32.DWORD
 
 	__windows_thread_entry_proc :: proc "stdcall" (t_: rawptr) -> win32.DWORD {
 		t := (^Thread)(t_)
-		context = t.init_context.? or_else runtime.default_context()
-		
+
 		t.id = sync.current_thread_id()
 
-		t.procedure(t)
+		{
+			init_context := t.init_context
+
+			// NOTE(tetra, 2023-05-31): Must do this AFTER thread.start() is called, so that the user can set the init_context, etc!
+			// Here on Windows, the thread is created in a suspended state, and so we can select the context anywhere before the call
+			// to t.procedure().
+			context = _select_context_for_thread(init_context)
+			defer _maybe_destroy_default_temp_allocator(init_context)
+
+			t.procedure(t)
+		}
 
 		intrinsics.atomic_store(&t.flags, t.flags + {.Done})
 
-		if t.init_context == nil {
-			if context.temp_allocator.data == &runtime.global_default_temp_allocator_data {
-				runtime.default_temp_allocator_destroy(auto_cast context.temp_allocator.data)
-			}
-		}
 		return 0
 	}