Skip to content

Commit 916bb88

Browse files
authored
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 <[email protected]>
1 parent 84ac47b commit 916bb88

File tree

4 files changed

+0
-27
lines changed

4 files changed

+0
-27
lines changed

runtime/src/iree/base/internal/threading_darwin.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ static void* iree_thread_start_routine(void* param) {
7777
thread->entry = NULL;
7878
thread->entry_arg = NULL;
7979

80-
// Release our ownership of the thread handle. If the creating thread doesn't
81-
// want it this will free the memory and fully detach the thread.
82-
iree_thread_release(thread);
83-
8480
// Call the user thread entry point function.
8581
// Note that this can be a tail-call which saves a stack frame in all threads
8682
// (which is really just to make call stacks in debuggers much cleaner).
@@ -128,9 +124,6 @@ iree_status_t iree_thread_create(iree_thread_entry_t entry, void* entry_arg,
128124
}
129125
pthread_attr_set_qos_class_np(&thread_attr, qos_class, 0);
130126

131-
// Retain the thread for the thread itself; this way if the caller immediately
132-
// releases the iree_thread_t handle the thread won't explode.
133-
iree_thread_retain(thread);
134127
*out_thread = thread;
135128

136129
// Create the thread either suspended or running as the user requested.
@@ -148,7 +141,6 @@ iree_status_t iree_thread_create(iree_thread_entry_t entry, void* entry_arg,
148141
}
149142
pthread_attr_destroy(&thread_attr);
150143
if (rc != 0) {
151-
iree_thread_release(thread); // for self
152144
iree_thread_release(thread); // for caller
153145
*out_thread = NULL;
154146
IREE_TRACE_ZONE_END(z0);

runtime/src/iree/base/internal/threading_pthreads.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,6 @@ static void* iree_thread_start_routine(void* param) {
113113
thread->entry = NULL;
114114
thread->entry_arg = NULL;
115115

116-
// Release our ownership of the thread handle. If the creating thread doesn't
117-
// want it this will free the memory and fully detach the thread.
118-
iree_thread_release(thread);
119-
120116
// Call the user thread entry point function.
121117
// Note that this can be a tail-call which saves a stack frame in all threads
122118
// (which is really just to make call stacks in debuggers much cleaner).
@@ -157,9 +153,6 @@ iree_status_t iree_thread_create(iree_thread_entry_t entry, void* entry_arg,
157153
pthread_attr_setstacksize(&thread_attr, params.stack_size);
158154
}
159155

160-
// Retain the thread for the thread itself; this way if the caller immediately
161-
// releases the iree_thread_t handle the thread won't explode.
162-
iree_thread_retain(thread);
163156
*out_thread = thread;
164157

165158
// Unfortunately we can't create the thread suspended (no API). This means
@@ -176,7 +169,6 @@ iree_status_t iree_thread_create(iree_thread_entry_t entry, void* entry_arg,
176169
}
177170
pthread_attr_destroy(&thread_attr);
178171
if (rc != 0) {
179-
iree_thread_release(thread); // for self
180172
iree_thread_release(thread); // for caller
181173
*out_thread = NULL;
182174
IREE_TRACE_ZONE_END(z0);

runtime/src/iree/base/internal/threading_win32.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,6 @@ static DWORD WINAPI iree_thread_start_routine(LPVOID param) {
116116
thread->entry = NULL;
117117
thread->entry_arg = NULL;
118118

119-
// Release our ownership of the thread handle. If the creating thread doesn't
120-
// want it this will free the memory and fully detach the thread.
121-
iree_thread_release(thread);
122-
123119
// Call the user thread entry point function.
124120
// Note that this can be a tail-call which saves a stack frame in all threads
125121
// (which is really just to make call stacks in debuggers much cleaner).
@@ -154,9 +150,6 @@ iree_status_t iree_thread_create(iree_thread_entry_t entry, void* entry_arg,
154150
params.priority_class, thread->allocator,
155151
&thread->qos_override_list);
156152

157-
// Retain the thread for the thread itself; this way if the caller immediately
158-
// releases the iree_thread_t handle the thread won't explode.
159-
iree_thread_retain(thread);
160153
*out_thread = thread;
161154

162155
// Create the thread either suspended or running as the user requested.
@@ -169,7 +162,6 @@ iree_status_t iree_thread_create(iree_thread_entry_t entry, void* entry_arg,
169162
}
170163
if (thread->handle == INVALID_HANDLE_VALUE) {
171164
iree_thread_release(thread); // for self
172-
iree_thread_release(thread); // for caller
173165
*out_thread = NULL;
174166
IREE_TRACE_ZONE_END(z0);
175167
return iree_make_status(IREE_STATUS_INTERNAL,

runtime/src/iree/hal/utils/deferred_work_queue.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -598,10 +598,7 @@ void iree_hal_deferred_work_queue_destroy(
598598
// Request the workers to exit.
599599
iree_hal_deferred_work_queue_request_exit(work_queue);
600600

601-
iree_thread_join(work_queue->worker_thread);
602601
iree_thread_release(work_queue->worker_thread);
603-
604-
iree_thread_join(work_queue->completion_thread);
605602
iree_thread_release(work_queue->completion_thread);
606603

607604
iree_hal_deferred_work_queue_working_area_deinitialize(

0 commit comments

Comments
 (0)