Skip to content

Commit 0ac9c68

Browse files
committed
change locks to avoid capturing pthread_self value
It was observed in TSAN that we might get incorrectly locked mutexes when this code is inlined, due to pthread_self being normally marked const. (cherry-picked from 95fac8f92dZ)
1 parent 28b7b06 commit 0ac9c68

File tree

4 files changed

+74
-78
lines changed

4 files changed

+74
-78
lines changed

src/jltypes.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,13 +2623,8 @@ void jl_init_types(void) JL_GC_DISABLED
26232623
jl_svecset(jl_methtable_type->types, 4, jl_long_type);
26242624
jl_svecset(jl_methtable_type->types, 6, jl_module_type);
26252625
jl_svecset(jl_methtable_type->types, 7, jl_array_any_type);
2626-
#ifdef __LP64__
2627-
jl_svecset(jl_methtable_type->types, 8, jl_int64_type); // unsigned long
2628-
jl_svecset(jl_methtable_type->types, 9, jl_int64_type); // uint32_t plus alignment
2629-
#else
2630-
jl_svecset(jl_methtable_type->types, 8, jl_int32_type); // DWORD
2631-
jl_svecset(jl_methtable_type->types, 9, jl_int32_type); // uint32_t
2632-
#endif
2626+
jl_svecset(jl_methtable_type->types, 8, jl_long_type); // jl_task_t*
2627+
jl_svecset(jl_methtable_type->types, 9, jl_long_type); // uint32_t plus alignment
26332628
jl_svecset(jl_methtable_type->types, 10, jl_uint8_type);
26342629
jl_svecset(jl_methtable_type->types, 11, jl_uint8_type);
26352630
jl_svecset(jl_method_type->types, 12, jl_method_instance_type);

src/julia_threads.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ typedef pthread_t jl_thread_t;
9797

9898
// Recursive spin lock
9999
typedef struct {
100-
volatile jl_thread_t owner;
100+
struct _jl_task_t *owner;
101101
uint32_t count;
102102
} jl_mutex_t;
103103

src/locks.h

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,27 @@ extern "C" {
1111

1212
// Lock acquire and release primitives
1313

14-
// JL_LOCK and jl_mutex_lock are GC safe points while JL_LOCK_NOGC
15-
// and jl_mutex_lock_nogc are not.
14+
// JL_LOCK and jl_mutex_lock are GC safe points, use uv_mutex_t if that is not desired.
1615
// Always use JL_LOCK unless no one holding the lock can trigger a GC or GC
17-
// safepoint. JL_LOCK_NOGC should only be needed for GC internal locks.
16+
// safepoint. uv_mutex_t should only be needed for GC internal locks.
1817
// The JL_LOCK* and JL_UNLOCK* macros are no-op for non-threading build
1918
// while the jl_mutex_* functions are always locking and unlocking the locks.
2019

2120
static inline void jl_mutex_wait(jl_mutex_t *lock, int safepoint)
2221
{
23-
jl_thread_t self = jl_thread_self();
24-
jl_thread_t owner = jl_atomic_load_relaxed(&lock->owner);
25-
jl_task_t *ct = jl_current_task;
22+
jl_task_t *self = jl_current_task;
23+
jl_task_t *owner = jl_atomic_load_relaxed(&lock->owner);
2624
if (owner == self) {
2725
lock->count++;
2826
return;
2927
}
3028
while (1) {
31-
if (owner == 0 && jl_atomic_cmpswap(&lock->owner, &owner, self)) {
29+
if (owner == NULL && jl_atomic_cmpswap(&lock->owner, &owner, self)) {
3230
lock->count = 1;
3331
return;
3432
}
3533
if (safepoint) {
36-
jl_gc_safepoint_(ct->ptls);
34+
jl_gc_safepoint_(self->ptls);
3735
}
3836
jl_cpu_pause();
3937
owner = jl_atomic_load_relaxed(&lock->owner);
@@ -90,13 +88,13 @@ static inline void jl_mutex_lock(jl_mutex_t *lock)
9088

9189
static inline int jl_mutex_trylock_nogc(jl_mutex_t *lock)
9290
{
93-
jl_thread_t self = jl_thread_self();
94-
jl_thread_t owner = jl_atomic_load_acquire(&lock->owner);
91+
jl_task_t *self = jl_current_task;
92+
jl_task_t *owner = jl_atomic_load_acquire(&lock->owner);
9593
if (owner == self) {
9694
lock->count++;
9795
return 1;
9896
}
99-
if (owner == 0 && jl_atomic_cmpswap(&lock->owner, &owner, self)) {
97+
if (owner == NULL && jl_atomic_cmpswap(&lock->owner, &owner, self)) {
10098
lock->count = 1;
10199
return 1;
102100
}
@@ -115,10 +113,10 @@ static inline int jl_mutex_trylock(jl_mutex_t *lock)
115113
static inline void jl_mutex_unlock_nogc(jl_mutex_t *lock) JL_NOTSAFEPOINT
116114
{
117115
#ifndef __clang_analyzer__
118-
assert(lock->owner == jl_thread_self() &&
116+
assert(jl_atomic_load_relaxed(&lock->owner) == jl_current_task &&
119117
"Unlocking a lock in a different thread.");
120118
if (--lock->count == 0) {
121-
jl_atomic_store_release(&lock->owner, 0);
119+
jl_atomic_store_release(&lock->owner, (jl_task_t*)NULL);
122120
jl_cpu_wake();
123121
}
124122
#endif
@@ -129,14 +127,14 @@ static inline void jl_mutex_unlock(jl_mutex_t *lock)
129127
jl_mutex_unlock_nogc(lock);
130128
jl_lock_frame_pop();
131129
JL_SIGATOMIC_END();
132-
if (jl_gc_have_pending_finalizers) {
130+
if (jl_atomic_load_relaxed(&jl_gc_have_pending_finalizers)) {
133131
jl_gc_run_pending_finalizers(jl_current_task); // may GC
134132
}
135133
}
136134

137135
static inline void jl_mutex_init(jl_mutex_t *lock) JL_NOTSAFEPOINT
138136
{
139-
lock->owner = 0;
137+
jl_atomic_store_relaxed(&lock->owner, (jl_task_t*)NULL);
140138
lock->count = 0;
141139
}
142140

0 commit comments

Comments
 (0)