Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wipe ptls->system_id when a thread exits. #50747

Closed
wants to merge 3 commits into from
Closed

Wipe ptls->system_id when a thread exits. #50747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e861fb
Wipe ptls->system_id when a thread exits.
maleadt Aug 1, 2023
288eeaf
Make system_id accesses atomic.
maleadt Aug 1, 2023
37ca3f9
Fix data race.
maleadt Aug 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/julia_threads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ typedef struct _jl_tls_states_t {
#else
void *signal_stack;
#endif
jl_thread_t system_id;
_Atomic(jl_thread_t) system_id;
arraylist_t finalizers;
struct _jl_gc_pagemeta_t *page_metadata_allocd;
struct _jl_gc_pagemeta_t *page_metadata_lazily_freed;
Expand Down
13 changes: 6 additions & 7 deletions src/signals-mach.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void jl_mach_gc_end(void)
int8_t gc_state = (int8_t)(item >> 8);
jl_ptls_t ptls2 = jl_atomic_load_relaxed(&jl_all_tls_states)[tid];
jl_atomic_store_release(&ptls2->gc_state, gc_state);
thread_resume(pthread_mach_thread_np(ptls2->system_id));
thread_resume(pthread_mach_thread_np(jl_atomic_load_relaxed(&ptls2->system_id)));
}
suspended_threads.len = 0;
}
Expand Down Expand Up @@ -279,7 +279,7 @@ kern_return_t catch_mach_exception_raise(
int nthreads = jl_atomic_load_acquire(&jl_n_threads);
for (tid = 0; tid < nthreads; tid++) {
jl_ptls_t _ptls2 = jl_atomic_load_relaxed(&jl_all_tls_states)[tid];
if (pthread_mach_thread_np(_ptls2->system_id) == thread) {
if (pthread_mach_thread_np(jl_atomic_load_relaxed(&_ptls2->system_id)) == thread) {
maleadt marked this conversation as resolved.
Show resolved Hide resolved
ptls2 = _ptls2;
break;
}
Expand Down Expand Up @@ -385,12 +385,11 @@ static int jl_thread_suspend_and_get_state2(int tid, host_thread_state_t *ctx)
jl_ptls_t ptls2 = jl_atomic_load_relaxed(&jl_all_tls_states)[tid];
if (ptls2 == NULL) // this thread is not alive
return 0;
mach_port_t thread = pthread_mach_thread_np(jl_atomic_load_acquire(&ptls2->system_id));
jl_task_t *ct2 = ptls2 ? jl_atomic_load_relaxed(&ptls2->current_task) : NULL;
if (ct2 == NULL) // this thread is already dead
return 0;

mach_port_t thread = pthread_mach_thread_np(ptls2->system_id);

kern_return_t ret = thread_suspend(thread);
HANDLE_MACH_ERROR("thread_suspend", ret);

Expand All @@ -417,7 +416,7 @@ static void jl_thread_suspend_and_get_state(int tid, int timeout, unw_context_t
static void jl_thread_resume(int tid, int sig)
{
jl_ptls_t ptls2 = jl_atomic_load_relaxed(&jl_all_tls_states)[tid];
mach_port_t thread = pthread_mach_thread_np(ptls2->system_id);
mach_port_t thread = pthread_mach_thread_np(jl_atomic_load_relaxed(&ptls2->system_id));
kern_return_t ret = thread_resume(thread);
HANDLE_MACH_ERROR("thread_resume", ret);
}
Expand All @@ -427,7 +426,7 @@ static void jl_thread_resume(int tid, int sig)
static void jl_try_deliver_sigint(void)
{
jl_ptls_t ptls2 = jl_atomic_load_relaxed(&jl_all_tls_states)[0];
mach_port_t thread = pthread_mach_thread_np(ptls2->system_id);
mach_port_t thread = pthread_mach_thread_np(jl_atomic_load_relaxed(&ptls2->system_id));

kern_return_t ret = thread_suspend(thread);
HANDLE_MACH_ERROR("thread_suspend", ret);
Expand Down Expand Up @@ -464,7 +463,7 @@ CFI_NORETURN
static void jl_exit_thread0(int signo, jl_bt_element_t *bt_data, size_t bt_size)
{
jl_ptls_t ptls2 = jl_atomic_load_relaxed(&jl_all_tls_states)[0];
mach_port_t thread = pthread_mach_thread_np(ptls2->system_id);
mach_port_t thread = pthread_mach_thread_np(jl_atomic_load_relaxed(&ptls2->system_id));

host_thread_state_t state;
if (!jl_thread_suspend_and_get_state2(0, &state)) {
Expand Down
6 changes: 3 additions & 3 deletions src/signals-unix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ static void jl_thread_suspend_and_get_state(int tid, int timeout, unw_context_t
return;
}
jl_atomic_store_release(&ptls2->signal_request, 1);
pthread_kill(ptls2->system_id, SIGUSR2);
pthread_kill(jl_atomic_load_relaxed(&ptls2->system_id), SIGUSR2);
// wait for thread to acknowledge
int err = pthread_cond_timedwait(&signal_caught_cond, &in_signal_lock, &ts);
if (err == ETIMEDOUT) {
Expand Down Expand Up @@ -455,7 +455,7 @@ static void jl_try_deliver_sigint(void)
jl_wake_libuv();
jl_atomic_store_release(&ptls2->signal_request, 2);
// This also makes sure `sleep` is aborted.
pthread_kill(ptls2->system_id, SIGUSR2);
pthread_kill(jl_atomic_load_relaxed(&ptls2->system_id), SIGUSR2);
}

// Write only by signal handling thread, read only by main thread
Expand Down Expand Up @@ -640,7 +640,7 @@ void jl_install_thread_signal_handler(jl_ptls_t ptls)
}

#ifdef HAVE_MACH
attach_exception_port(pthread_mach_thread_np(ptls->system_id), 0);
attach_exception_port(pthread_mach_thread_np(jl_atomic_load_relaxed(&ptls->system_id)), 0);
#endif
}

Expand Down
6 changes: 4 additions & 2 deletions src/threading.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ jl_ptls_t jl_init_threadtls(int16_t tid)
#ifndef _OS_WINDOWS_
pthread_setspecific(jl_task_exit_key, (void*)ptls);
#endif
ptls->system_id = (jl_thread_t)(uintptr_t)uv_thread_self();
jl_atomic_store_relaxed(&ptls->system_id, (jl_thread_t)(uintptr_t)uv_thread_self());
ptls->rngseed = jl_rand();
if (tid == 0)
ptls->disable_gc = 1;
Expand Down Expand Up @@ -473,7 +473,9 @@ static void jl_delete_thread(void *value) JL_NOTSAFEPOINT_ENTER
jl_safe_printf("fatal: thread exited from wrong Task.\n");
abort();
}
jl_atomic_store_relaxed(&ptls->current_task, NULL); // dead
// mark this thread as dead
jl_atomic_store_relaxed(&ptls->current_task, NULL);
jl_atomic_store_release(&ptls->system_id, 0);
// finally, release all of the locks we had grabbed
#ifdef _OS_WINDOWS_
jl_unlock_profile_wr();
Expand Down