Skip to content

Commit 916f4c2

Browse files
vtjnashKristofferC
authored andcommitted
avoid deadlock if crashing inside profile_wr_lock (#58452)
The rd/wr lock distinction here was supposed to help prevent deadlocks by allowing recursion (even over signals), but did not account for crashes causing recursion while holding the wr lock. Make these lock acquires fail-able if they would cause deadlock. (cherry picked from commit 2e2fac5)
1 parent ab198f9 commit 916f4c2

File tree

4 files changed

+55
-21
lines changed

4 files changed

+55
-21
lines changed

src/debuginfo.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ struct unw_table_entry
145145
template <typename T>
146146
static void jl_profile_atomic(T f) JL_NOTSAFEPOINT
147147
{
148-
assert(0 == jl_lock_profile_rd_held());
149-
jl_lock_profile_wr();
148+
int havelock = jl_lock_profile_wr();
149+
assert(havelock);
150150
#ifndef _OS_WINDOWS_
151151
sigset_t sset;
152152
sigset_t oset;
@@ -157,7 +157,8 @@ static void jl_profile_atomic(T f) JL_NOTSAFEPOINT
157157
#ifndef _OS_WINDOWS_
158158
pthread_sigmask(SIG_SETMASK, &oset, NULL);
159159
#endif
160-
jl_unlock_profile_wr();
160+
if (havelock)
161+
jl_unlock_profile_wr();
161162
}
162163

163164

@@ -464,8 +465,8 @@ static int lookup_pointer(
464465

465466
// DWARFContext/DWARFUnit update some internal tables during these queries, so
466467
// a lock is needed.
467-
assert(0 == jl_lock_profile_rd_held());
468-
jl_lock_profile_wr();
468+
if (!jl_lock_profile_wr())
469+
return lookup_pointer(object::SectionRef(), NULL, frames, pointer, slide, demangle, noInline);
469470
auto inlineInfo = context->getInliningInfoForAddress(makeAddress(Section, pointer + slide), infoSpec);
470471
jl_unlock_profile_wr();
471472

@@ -490,7 +491,8 @@ static int lookup_pointer(
490491
info = inlineInfo.getFrame(i);
491492
}
492493
else {
493-
jl_lock_profile_wr();
494+
int havelock = jl_lock_profile_wr();
495+
assert(havelock); (void)havelock;
494496
info = context->getLineInfoForAddress(makeAddress(Section, pointer + slide), infoSpec);
495497
jl_unlock_profile_wr();
496498
}
@@ -1195,8 +1197,8 @@ int jl_DI_for_fptr(uint64_t fptr, uint64_t *symsize, int64_t *slide,
11951197
object::SectionRef *Section, llvm::DIContext **context) JL_NOTSAFEPOINT
11961198
{
11971199
int found = 0;
1198-
assert(0 == jl_lock_profile_rd_held());
1199-
jl_lock_profile_wr();
1200+
if (!jl_lock_profile_wr())
1201+
return 0;
12001202

12011203
if (symsize)
12021204
*symsize = 0;

src/julia_internal.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,9 @@ JL_DLLEXPORT double jl_get_profile_peek_duration(void);
207207
JL_DLLEXPORT void jl_set_profile_peek_duration(double);
208208

209209
JL_DLLEXPORT void jl_init_profile_lock(void);
210-
JL_DLLEXPORT uintptr_t jl_lock_profile_rd_held(void) JL_NOTSAFEPOINT;
211-
JL_DLLEXPORT void jl_lock_profile(void) JL_NOTSAFEPOINT JL_NOTSAFEPOINT_ENTER;
210+
JL_DLLEXPORT int jl_lock_profile(void) JL_NOTSAFEPOINT JL_NOTSAFEPOINT_ENTER;
212211
JL_DLLEXPORT void jl_unlock_profile(void) JL_NOTSAFEPOINT JL_NOTSAFEPOINT_LEAVE;
213-
JL_DLLEXPORT void jl_lock_profile_wr(void) JL_NOTSAFEPOINT JL_NOTSAFEPOINT_ENTER;
212+
JL_DLLEXPORT int jl_lock_profile_wr(void) JL_NOTSAFEPOINT JL_NOTSAFEPOINT_ENTER;
214213
JL_DLLEXPORT void jl_unlock_profile_wr(void) JL_NOTSAFEPOINT JL_NOTSAFEPOINT_LEAVE;
215214
void jl_with_stackwalk_lock(void (*f)(void*) JL_NOTSAFEPOINT, void *ctx) JL_NOTSAFEPOINT;
216215

src/signal-handling.c

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ void jl_init_profile_lock(void)
102102
#endif
103103
}
104104

105-
uintptr_t jl_lock_profile_rd_held(void)
105+
static uintptr_t jl_lock_profile_rd_held(void) JL_NOTSAFEPOINT
106106
{
107107
#ifndef _OS_WINDOWS_
108108
return (uintptr_t)pthread_getspecific(debuginfo_asyncsafe_held);
@@ -111,38 +111,69 @@ uintptr_t jl_lock_profile_rd_held(void)
111111
#endif
112112
}
113113

114-
void jl_lock_profile(void)
114+
int jl_lock_profile(void)
115115
{
116116
uintptr_t held = jl_lock_profile_rd_held();
117-
if (held++ == 0)
117+
if (held == -1)
118+
return 0;
119+
if (held == 0) {
120+
held = -1;
121+
#ifndef _OS_WINDOWS_
122+
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
123+
#else
124+
TlsSetValue(debuginfo_asyncsafe_held, (void*)held);
125+
#endif
118126
uv_rwlock_rdlock(&debuginfo_asyncsafe);
127+
held = 0;
128+
}
129+
held++;
119130
#ifndef _OS_WINDOWS_
120131
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
121132
#else
122133
TlsSetValue(debuginfo_asyncsafe_held, (void*)held);
123134
#endif
135+
return 1;
124136
}
125137

126138
JL_DLLEXPORT void jl_unlock_profile(void)
127139
{
128140
uintptr_t held = jl_lock_profile_rd_held();
129-
assert(held);
130-
if (--held == 0)
131-
uv_rwlock_rdunlock(&debuginfo_asyncsafe);
141+
assert(held && held != -1);
142+
held--;
132143
#ifndef _OS_WINDOWS_
133144
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
134145
#else
135146
TlsSetValue(debuginfo_asyncsafe_held, (void*)held);
136147
#endif
148+
if (held == 0)
149+
uv_rwlock_rdunlock(&debuginfo_asyncsafe);
137150
}
138151

139-
void jl_lock_profile_wr(void)
152+
int jl_lock_profile_wr(void)
140153
{
154+
uintptr_t held = jl_lock_profile_rd_held();
155+
if (held)
156+
return 0;
157+
held = -1;
158+
#ifndef _OS_WINDOWS_
159+
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
160+
#else
161+
TlsSetValue(debuginfo_asyncsafe_held, (void*)held);
162+
#endif
141163
uv_rwlock_wrlock(&debuginfo_asyncsafe);
164+
return 1;
142165
}
143166

144167
void jl_unlock_profile_wr(void)
145168
{
169+
uintptr_t held = jl_lock_profile_rd_held();
170+
assert(held == -1);
171+
held = 0;
172+
#ifndef _OS_WINDOWS_
173+
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
174+
#else
175+
TlsSetValue(debuginfo_asyncsafe_held, (void*)held);
176+
#endif
146177
uv_rwlock_wrunlock(&debuginfo_asyncsafe);
147178
}
148179

src/threading.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -554,18 +554,20 @@ static void jl_delete_thread(void *value) JL_NOTSAFEPOINT_ENTER
554554
// this here by blocking. This also synchronizes our read of `current_task`
555555
// (which is the flag we currently use to check the liveness state of a thread).
556556
#ifdef _OS_WINDOWS_
557-
jl_lock_profile_wr();
557+
int havelock = jl_lock_profile_wr();
558+
assert(havelock); (void)havelock;
558559
#elif defined(JL_DISABLE_LIBUNWIND)
559560
// nothing
560561
#elif defined(__APPLE__)
561-
jl_lock_profile_wr();
562+
int havelock = jl_lock_profile_wr();
563+
assert(havelock); (void)havelock;
562564
#else
563565
pthread_mutex_lock(&in_signal_lock);
564566
#endif
565567
jl_atomic_store_relaxed(&ptls->current_task, NULL); // indicate dead
566568
// finally, release all of the locks we had grabbed
567569
#ifdef _OS_WINDOWS_
568-
jl_unlock_profile_wr();
570+
if (havelock) jl_unlock_profile_wr();
569571
#elif defined(JL_DISABLE_LIBUNWIND)
570572
// nothing
571573
#elif defined(__APPLE__)

0 commit comments

Comments
 (0)