Stopped threads from holding a reference to themselves. (#18636)
This caused a weird race condition, where if the owner of the thread
released it before it was even started, then the owner would continue
on, however only THEN would the thread actually start running at some
future time. It was also causing pthread_join to be called by the owning
thread.
This also fixes `deferred_action_queue` to not manually join the thread.
---------
Signed-off-by: Andrew Woloszyn <andrew.woloszyn@gmail.com>
diff --git a/runtime/src/iree/base/internal/threading_darwin.c b/runtime/src/iree/base/internal/threading_darwin.c
index 537f705..52932f8 100644
--- a/runtime/src/iree/base/internal/threading_darwin.c
+++ b/runtime/src/iree/base/internal/threading_darwin.c
@@ -77,10 +77,6 @@
thread->entry = NULL;
thread->entry_arg = NULL;
- // Release our ownership of the thread handle. If the creating thread doesn't
- // want it this will free the memory and fully detach the thread.
- iree_thread_release(thread);
-
// Call the user thread entry point function.
// Note that this can be a tail-call which saves a stack frame in all threads
// (which is really just to make call stacks in debuggers much cleaner).
@@ -128,9 +124,6 @@
}
pthread_attr_set_qos_class_np(&thread_attr, qos_class, 0);
- // Retain the thread for the thread itself; this way if the caller immediately
- // releases the iree_thread_t handle the thread won't explode.
- iree_thread_retain(thread);
*out_thread = thread;
// Create the thread either suspended or running as the user requested.
@@ -148,7 +141,6 @@
}
pthread_attr_destroy(&thread_attr);
if (rc != 0) {
- iree_thread_release(thread); // for self
iree_thread_release(thread); // for caller
*out_thread = NULL;
IREE_TRACE_ZONE_END(z0);
diff --git a/runtime/src/iree/base/internal/threading_pthreads.c b/runtime/src/iree/base/internal/threading_pthreads.c
index 0d5c016..1686fd1 100644
--- a/runtime/src/iree/base/internal/threading_pthreads.c
+++ b/runtime/src/iree/base/internal/threading_pthreads.c
@@ -113,10 +113,6 @@
thread->entry = NULL;
thread->entry_arg = NULL;
- // Release our ownership of the thread handle. If the creating thread doesn't
- // want it this will free the memory and fully detach the thread.
- iree_thread_release(thread);
-
// Call the user thread entry point function.
// Note that this can be a tail-call which saves a stack frame in all threads
// (which is really just to make call stacks in debuggers much cleaner).
@@ -157,9 +153,6 @@
pthread_attr_setstacksize(&thread_attr, params.stack_size);
}
- // Retain the thread for the thread itself; this way if the caller immediately
- // releases the iree_thread_t handle the thread won't explode.
- iree_thread_retain(thread);
*out_thread = thread;
// Unfortunately we can't create the thread suspended (no API). This means
@@ -176,7 +169,6 @@
}
pthread_attr_destroy(&thread_attr);
if (rc != 0) {
- iree_thread_release(thread); // for self
iree_thread_release(thread); // for caller
*out_thread = NULL;
IREE_TRACE_ZONE_END(z0);
diff --git a/runtime/src/iree/base/internal/threading_win32.c b/runtime/src/iree/base/internal/threading_win32.c
index 66e6a07..6166ce2 100644
--- a/runtime/src/iree/base/internal/threading_win32.c
+++ b/runtime/src/iree/base/internal/threading_win32.c
@@ -116,10 +116,6 @@
thread->entry = NULL;
thread->entry_arg = NULL;
- // Release our ownership of the thread handle. If the creating thread doesn't
- // want it this will free the memory and fully detach the thread.
- iree_thread_release(thread);
-
// Call the user thread entry point function.
// Note that this can be a tail-call which saves a stack frame in all threads
// (which is really just to make call stacks in debuggers much cleaner).
@@ -154,9 +150,6 @@
params.priority_class, thread->allocator,
&thread->qos_override_list);
- // Retain the thread for the thread itself; this way if the caller immediately
- // releases the iree_thread_t handle the thread won't explode.
- iree_thread_retain(thread);
*out_thread = thread;
// Create the thread either suspended or running as the user requested.
@@ -169,7 +162,6 @@
}
if (thread->handle == INVALID_HANDLE_VALUE) {
iree_thread_release(thread); // for self
- iree_thread_release(thread); // for caller
*out_thread = NULL;
IREE_TRACE_ZONE_END(z0);
return iree_make_status(IREE_STATUS_INTERNAL,
diff --git a/runtime/src/iree/hal/utils/deferred_work_queue.c b/runtime/src/iree/hal/utils/deferred_work_queue.c
index 22c229b..a0214b5 100644
--- a/runtime/src/iree/hal/utils/deferred_work_queue.c
+++ b/runtime/src/iree/hal/utils/deferred_work_queue.c
@@ -598,10 +598,7 @@
// Request the workers to exit.
iree_hal_deferred_work_queue_request_exit(work_queue);
- iree_thread_join(work_queue->worker_thread);
iree_thread_release(work_queue->worker_thread);
-
- iree_thread_join(work_queue->completion_thread);
iree_thread_release(work_queue->completion_thread);
iree_hal_deferred_work_queue_working_area_deinitialize(
