Skip to content

Commit ed7fb2a

Browse files
vtjnashKristofferC
authored andcommitted
profile: fix async deadlock (#44781)
Acquiring this lock in many implementations could result in deadlock, even with an existing reader. Add a TLS check for reentry before, instead of relying on the implementation specifics, to avoid any issues. (cherry picked from commit 7df454b)
1 parent ede6a24 commit ed7fb2a

File tree

1 file changed

+32
-16
lines changed

1 file changed

+32
-16
lines changed

src/debuginfo.cpp

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,29 +48,43 @@ typedef object::SymbolRef SymRef;
4848
// while holding this lock.
4949
// Certain functions in this file might be called from an unmanaged thread
5050
// and cannot have any interaction with the julia runtime
51-
static uv_rwlock_t threadsafe;
51+
// They also may be re-entrant, and operating while threads are paused, so we
52+
// separately manage the re-entrant count behavior for safety across platforms
53+
// Note that we cannot safely upgrade read->write
54+
static uv_rwlock_t debuginfo_asyncsafe;
55+
static pthread_key_t debuginfo_asyncsafe_held;
5256

5357
extern "C" void jl_init_debuginfo(void)
5458
{
55-
uv_rwlock_init(&threadsafe);
59+
uv_rwlock_init(&debuginfo_asyncsafe);
60+
if (pthread_key_create(&debuginfo_asyncsafe_held, NULL))
61+
jl_error("fatal: pthread_key_create failed");
5662
}
5763

5864
extern "C" void jl_lock_profile(void)
5965
{
60-
uv_rwlock_rdlock(&threadsafe);
66+
uintptr_t held = (uintptr_t)pthread_getspecific(debuginfo_asyncsafe_held);
67+
if (held++ == 0)
68+
uv_rwlock_rdlock(&debuginfo_asyncsafe);
69+
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
6170
}
6271

6372
extern "C" void jl_unlock_profile(void)
6473
{
65-
uv_rwlock_rdunlock(&threadsafe);
74+
uintptr_t held = (uintptr_t)pthread_getspecific(debuginfo_asyncsafe_held);
75+
assert(held);
76+
if (--held == 0)
77+
uv_rwlock_rdunlock(&debuginfo_asyncsafe);
78+
pthread_setspecific(debuginfo_asyncsafe_held, (void*)held);
6679
}
6780

6881
// some actions aren't signal (especially profiler) safe so we acquire a lock
6982
// around them to establish a mutual exclusion with unwinding from a signal
7083
template <typename T>
7184
static void jl_profile_atomic(T f)
7285
{
73-
uv_rwlock_wrlock(&threadsafe);
86+
assert(0 == (uintptr_t)pthread_getspecific(debuginfo_asyncsafe_held));
87+
uv_rwlock_wrlock(&debuginfo_asyncsafe);
7488
#ifndef _OS_WINDOWS_
7589
sigset_t sset;
7690
sigset_t oset;
@@ -81,7 +95,7 @@ static void jl_profile_atomic(T f)
8195
#ifndef _OS_WINDOWS_
8296
pthread_sigmask(SIG_SETMASK, &oset, NULL);
8397
#endif
84-
uv_rwlock_wrunlock(&threadsafe);
98+
uv_rwlock_wrunlock(&debuginfo_asyncsafe);
8599
}
86100

87101

@@ -197,12 +211,12 @@ class JuliaJITEventListener: public JITEventListener
197211

198212
jl_method_instance_t *lookupLinfo(size_t pointer) JL_NOTSAFEPOINT
199213
{
200-
uv_rwlock_rdlock(&threadsafe);
214+
jl_lock_profile();
201215
auto region = linfomap.lower_bound(pointer);
202216
jl_method_instance_t *linfo = NULL;
203217
if (region != linfomap.end() && pointer < region->first + region->second.first)
204218
linfo = region->second.second;
205-
uv_rwlock_rdunlock(&threadsafe);
219+
jl_unlock_profile();
206220
return linfo;
207221
}
208222

@@ -525,9 +539,10 @@ static int lookup_pointer(
525539

526540
// DWARFContext/DWARFUnit update some internal tables during these queries, so
527541
// a lock is needed.
528-
uv_rwlock_wrlock(&threadsafe);
542+
assert(0 == (uintptr_t)pthread_getspecific(debuginfo_asyncsafe_held));
543+
uv_rwlock_wrlock(&debuginfo_asyncsafe);
529544
auto inlineInfo = context->getInliningInfoForAddress(makeAddress(Section, pointer + slide), infoSpec);
530-
uv_rwlock_wrunlock(&threadsafe);
545+
uv_rwlock_wrunlock(&debuginfo_asyncsafe);
531546

532547
int fromC = (*frames)[0].fromC;
533548
int n_frames = inlineInfo.getNumberOfFrames();
@@ -550,9 +565,9 @@ static int lookup_pointer(
550565
info = inlineInfo.getFrame(i);
551566
}
552567
else {
553-
uv_rwlock_wrlock(&threadsafe);
568+
uv_rwlock_wrlock(&debuginfo_asyncsafe);
554569
info = context->getLineInfoForAddress(makeAddress(Section, pointer + slide), infoSpec);
555-
uv_rwlock_wrunlock(&threadsafe);
570+
uv_rwlock_wrunlock(&debuginfo_asyncsafe);
556571
}
557572

558573
jl_frame_t *frame = &(*frames)[i];
@@ -1208,7 +1223,8 @@ int jl_DI_for_fptr(uint64_t fptr, uint64_t *symsize, int64_t *slide,
12081223
object::SectionRef *Section, llvm::DIContext **context) JL_NOTSAFEPOINT
12091224
{
12101225
int found = 0;
1211-
uv_rwlock_wrlock(&threadsafe);
1226+
assert(0 == (uintptr_t)pthread_getspecific(debuginfo_asyncsafe_held));
1227+
uv_rwlock_wrlock(&debuginfo_asyncsafe);
12121228
std::map<size_t, ObjectInfo, revcomp> &objmap = jl_jit_events->getObjectMap();
12131229
std::map<size_t, ObjectInfo, revcomp>::iterator fit = objmap.lower_bound(fptr);
12141230

@@ -1224,7 +1240,7 @@ int jl_DI_for_fptr(uint64_t fptr, uint64_t *symsize, int64_t *slide,
12241240
}
12251241
found = 1;
12261242
}
1227-
uv_rwlock_wrunlock(&threadsafe);
1243+
uv_rwlock_wrunlock(&debuginfo_asyncsafe);
12281244
return found;
12291245
}
12301246

@@ -1669,13 +1685,13 @@ extern "C"
16691685
uint64_t jl_getUnwindInfo(uint64_t dwAddr)
16701686
{
16711687
// Might be called from unmanaged thread
1672-
uv_rwlock_rdlock(&threadsafe);
1688+
jl_lock_profile();
16731689
std::map<size_t, ObjectInfo, revcomp> &objmap = jl_jit_events->getObjectMap();
16741690
std::map<size_t, ObjectInfo, revcomp>::iterator it = objmap.lower_bound(dwAddr);
16751691
uint64_t ipstart = 0; // ip of the start of the section (if found)
16761692
if (it != objmap.end() && dwAddr < it->first + it->second.SectionSize) {
16771693
ipstart = (uint64_t)(uintptr_t)(*it).first;
16781694
}
1679-
uv_rwlock_rdunlock(&threadsafe);
1695+
jl_unlock_profile();
16801696
return ipstart;
16811697
}

0 commit comments

Comments
 (0)