Skip to content

Commit 580da4c

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Make JIT mutexes safe for fork
Summary: Currently we can have various JIT mutexes locked when a fork happens. This was a mostly a non-issue in the GIL build because Python code couldn't fork while the JIT was running. But w/ background compile it can. It would have been problematic if C code forked though. It's also presumably a lurking issue in the free-threaded builds. This makes it so we have proper safety for our locks around forking. Reviewed By: alexmalyshev Differential Revision: D115804360 fbshipit-source-id: adc835ed8787f64f36d6060dc3a242405a23e8ea
1 parent cd69203 commit 580da4c

13 files changed

Lines changed: 606 additions & 29 deletions

cinderx/Common/hugepages.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <cstdlib>
1717
#include <cstring>
1818
#include <memory>
19+
#include <new>
1920
#include <vector>
2021

2122
#ifndef WIN32
@@ -119,4 +120,19 @@ void HugePageArena::afterForkChild() {
119120
#endif
120121
}
121122

123+
void HugePageArena::atForkPrepare() {
124+
mutex_.lock();
125+
}
126+
127+
void HugePageArena::atForkParent() {
128+
mutex_.unlock();
129+
}
130+
131+
void HugePageArena::atForkChild() {
132+
// Reuse the storage to get a fresh, unlocked mutex. The inherited one is
133+
// still locked by atForkPrepare() and destroying a locked mutex is
134+
// undefined, so its lifetime is ended without running its destructor.
135+
new (&mutex_) std::mutex{};
136+
}
137+
122138
} // namespace cinderx

cinderx/Common/hugepages.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ class HugePageArena {
3535
// Re-establish huge page backing for every chunk after a fork().
3636
void afterForkChild();
3737

38+
void atForkPrepare();
39+
void atForkParent();
40+
void atForkChild();
41+
3842
private:
3943
struct Chunk {
4044
void* ptr;

cinderx/Common/slab_arena.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
#include "cinderx/module_state.h"
66

7+
#include <algorithm>
8+
#include <new>
9+
710
#if defined(__linux__) && defined(__aarch64__)
811
// On ARM64 we see huge dTLB misses on our inline caches so
912
// we put them on huge pages
@@ -22,4 +25,51 @@ std::shared_ptr<HugePageArena> getSharedHugePageArena() {
2225
return nullptr;
2326
}
2427

28+
SlabArenaForkRegistry& SlabArenaForkRegistry::get() {
29+
// Deliberately leaked: SlabArenas are owned by the CinderX module state,
30+
// which can outlive static destructors and would then unregister into a
31+
// destroyed object.
32+
static auto* registry = new SlabArenaForkRegistry;
33+
return *registry;
34+
}
35+
36+
void SlabArenaForkRegistry::add(std::mutex* mutex) {
37+
std::lock_guard<std::mutex> guard{lock_};
38+
mutexes_.push_back(mutex);
39+
}
40+
41+
void SlabArenaForkRegistry::remove(std::mutex* mutex) {
42+
auto it = std::find(mutexes_.begin(), mutexes_.end(), mutex);
43+
if (it != mutexes_.end()) {
44+
*it = std::move(mutexes_.back());
45+
mutexes_.pop_back();
46+
}
47+
}
48+
49+
void SlabArenaForkRegistry::atForkPrepare() {
50+
// Holding lock_ across the fork also stops the list itself from being
51+
// mutated while it's being walked.
52+
lock_.lock();
53+
for (std::mutex* mutex : mutexes_) {
54+
mutex->lock();
55+
}
56+
}
57+
58+
void SlabArenaForkRegistry::atForkParent() {
59+
for (std::mutex* mutex : mutexes_) {
60+
mutex->unlock();
61+
}
62+
lock_.unlock();
63+
}
64+
65+
void SlabArenaForkRegistry::atForkChild() {
66+
// Reuse each mutex's storage to get a fresh, unlocked one. They're all
67+
// still locked by atForkPrepare() and destroying a locked mutex is
68+
// undefined, so their lifetimes end without running their destructors.
69+
for (std::mutex* mutex : mutexes_) {
70+
new (mutex) std::mutex{};
71+
}
72+
new (&lock_) std::mutex{};
73+
}
74+
2575
} // namespace cinderx

cinderx/Common/slab_arena.h

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,28 @@ class SlabArenaIterator {
9292

9393
std::shared_ptr<HugePageArena> getSharedHugePageArena();
9494

95+
// The mutexes of every live SlabArena, so that pthread_atfork() handlers can
96+
// quiesce them across a fork().
97+
//
98+
// SlabArena is a template with instances scattered across JIT state, so they
99+
// register themselves here instead of being enumerated by hand. No SlabArena
100+
// ever locks another, so the handlers may take them in any order.
101+
class SlabArenaForkRegistry {
102+
public:
103+
static SlabArenaForkRegistry& get();
104+
105+
void add(std::mutex* mutex);
106+
void remove(std::mutex* mutex);
107+
108+
void atForkPrepare();
109+
void atForkParent();
110+
void atForkChild();
111+
112+
private:
113+
std::mutex lock_;
114+
std::vector<std::mutex*> mutexes_;
115+
};
116+
95117
// SlabArena is a simple arena allocator, using slabs that are multiples of the
96118
// system's page size. Allocated objects never move after creation, and all
97119
// objects will be kept alive until the SlabArena they came from is destroyed.
@@ -118,8 +140,20 @@ class SlabArena {
118140

119141
SlabArena() {
120142
slabs_.emplace_back(SizeTrait::size(), getSharedHugePageArena());
143+
// Registered last so a throwing constructor can't leave a dangling pointer
144+
// behind, as the destructor won't run for a half-constructed arena.
145+
SlabArenaForkRegistry::get().add(&mutex_);
121146
}
122147

148+
~SlabArena() {
149+
SlabArenaForkRegistry::get().remove(&mutex_);
150+
}
151+
152+
SlabArena(const SlabArena&) = delete;
153+
SlabArena(SlabArena&&) = delete;
154+
SlabArena& operator=(const SlabArena&) = delete;
155+
SlabArena& operator=(SlabArena&&) = delete;
156+
123157
// Allocate a new instance of T using the given constructor arguments.
124158
template <typename... Args>
125159
T* allocate(Args&&... args) {

cinderx/Jit/code_allocator.cpp

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "cinderx/Jit/codegen/code_section.h"
77
#include "cinderx/Jit/config.h"
88
#include "cinderx/Jit/jit_rt.h"
9+
#include "cinderx/module_state.h"
910

1011
#ifdef WIN32
1112
#include <Windows.h>
@@ -22,6 +23,7 @@
2223
#endif
2324

2425
#include <cstring>
26+
#include <new>
2527

2628
namespace cinderx::jit {
2729

@@ -211,6 +213,8 @@ ICodeAllocator* CodeAllocator::make() {
211213
}
212214

213215
AllocateResult CodeAllocator::addCode(asmjit::CodeHolder* code) {
216+
std::lock_guard lock{runtime_mutex_};
217+
214218
void* addr = nullptr;
215219
asmjit::Error error = runtime_.add(&addr, code);
216220

@@ -222,6 +226,8 @@ AllocateResult CodeAllocator::addCode(asmjit::CodeHolder* code) {
222226
}
223227

224228
asmjit::Error CodeAllocator::releaseCode(void* code) {
229+
std::lock_guard lock{runtime_mutex_};
230+
225231
// Find the size of the allocated region.
226232
asmjit::JitAllocator* inner = runtime_.allocator();
227233
asmjit::JitAllocator::Span span;
@@ -247,9 +253,12 @@ asmjit::Error CodeAllocator::releaseCode(void* code) {
247253
}
248254

249255
bool CodeAllocator::contains(const void* ptr) const {
256+
// query() is internally thread-safe, but it takes asmjit's own lock, which
257+
// can't be recovered in a forked child. Going through runtime_mutex_ keeps
258+
// that lock free whenever a fork can happen.
259+
std::lock_guard lock{runtime_mutex_};
260+
250261
asmjit::JitAllocator::Span unused;
251-
// asmjit docs don't say that query() is thread-safe, but peeking at the
252-
// implementation shows that it is.
253262
return runtime_.allocator()->query(unused, const_cast<void*>(ptr)) ==
254263
asmjit::kErrorOk;
255264
}
@@ -262,6 +271,21 @@ const asmjit::Environment& CodeAllocator::asmJitEnvironment() const {
262271
return runtime_.environment();
263272
}
264273

274+
void CodeAllocator::atForkPrepare() {
275+
runtime_mutex_.lock();
276+
}
277+
278+
void CodeAllocator::atForkParent() {
279+
runtime_mutex_.unlock();
280+
}
281+
282+
void CodeAllocator::atForkChild() {
283+
// Reuse the storage to get a fresh, unlocked mutex. The inherited one is
284+
// still locked by atForkPrepare() and destroying a locked mutex is
285+
// undefined, so its lifetime is ended without running its destructor.
286+
new (&runtime_mutex_) std::mutex{};
287+
}
288+
265289
CodeAllocatorCinder::~CodeAllocatorCinder() {
266290
for (std::span<uint8_t> alloc : allocations_) {
267291
#ifndef WIN32
@@ -484,4 +508,50 @@ bool CodeAllocatorCinder::contains(const void* ptr) const {
484508
return false;
485509
}
486510

511+
void CodeAllocatorCinder::atForkPrepare() {
512+
CodeAllocator::atForkPrepare();
513+
allocator_mutex_.lock();
514+
}
515+
516+
void CodeAllocatorCinder::atForkParent() {
517+
allocator_mutex_.unlock();
518+
CodeAllocator::atForkParent();
519+
}
520+
521+
void CodeAllocatorCinder::atForkChild() {
522+
new (&allocator_mutex_) std::mutex{};
523+
CodeAllocator::atForkChild();
524+
}
525+
526+
void codeAllocatorAtForkPrepare() {
527+
ModuleState* state = cinderx::getModuleState();
528+
if (state != nullptr && state->code_allocator != nullptr) {
529+
state->code_allocator->atForkPrepare();
530+
}
531+
#if defined(__linux__)
532+
// Innermost: ensureSpace() reaches this while holding allocator_mutex_.
533+
cinder_jit_region_mutex_.lock();
534+
#endif
535+
}
536+
537+
void codeAllocatorAtForkParent() {
538+
#if defined(__linux__)
539+
cinder_jit_region_mutex_.unlock();
540+
#endif
541+
ModuleState* state = cinderx::getModuleState();
542+
if (state != nullptr && state->code_allocator != nullptr) {
543+
state->code_allocator->atForkParent();
544+
}
545+
}
546+
547+
void codeAllocatorAtForkChild() {
548+
#if defined(__linux__)
549+
new (&cinder_jit_region_mutex_) std::mutex{};
550+
#endif
551+
ModuleState* state = cinderx::getModuleState();
552+
if (state != nullptr && state->code_allocator != nullptr) {
553+
state->code_allocator->atForkChild();
554+
}
555+
}
556+
487557
} // namespace cinderx::jit

cinderx/Jit/code_allocator.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,20 @@ class CodeAllocator : public ICodeAllocator {
4141
size_t usedBytes() const override;
4242
const asmjit::Environment& asmJitEnvironment() const override;
4343

44+
void atForkPrepare() override;
45+
void atForkParent() override;
46+
void atForkChild() override;
47+
4448
protected:
4549
asmjit::JitRuntime runtime_;
4650
std::atomic<size_t> used_bytes_{0};
51+
52+
// Serializes every operation that reaches asmjit's own JitAllocator lock.
53+
// That lock is private to asmjit with no way to reset it in a forked child,
54+
// so this makes it observably free at fork time instead: atForkPrepare()
55+
// holds this, which means no thread can be inside asmjit when the fork
56+
// happens.
57+
mutable std::mutex runtime_mutex_;
4758
};
4859

4960
// A code allocator which tries to allocate all code on huge pages.
@@ -71,6 +82,10 @@ class CodeAllocatorCinder : public CodeAllocator {
7182
asmjit::Error releaseCode(void* code) override;
7283
bool contains(const void* ptr) const override;
7384

85+
void atForkPrepare() override;
86+
void atForkParent() override;
87+
void atForkChild() override;
88+
7489
private:
7590
// Add code with hot/cold section splitting. Called by addCode() when
7691
// multiple_code_sections is enabled. Caller must hold allocator_mutex_.
@@ -114,6 +129,12 @@ class CodeAllocatorCinder : public CodeAllocator {
114129
std::atomic<size_t> fragmented_allocs_{0};
115130
};
116131

132+
// pthread_atfork() handlers covering both the process-global code-allocation
133+
// state and the current ICodeAllocator, if one exists.
134+
void codeAllocatorAtForkPrepare();
135+
void codeAllocatorAtForkParent();
136+
void codeAllocatorAtForkChild();
137+
117138
void populateCodeSections(
118139
std::vector<std::pair<void*, std::size_t>>& output_vector,
119140
asmjit::CodeHolder& code,

cinderx/Jit/code_allocator_iface.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ class ICodeAllocator {
3434

3535
// Get the asmjit environment used by this allocator.
3636
virtual const asmjit::Environment& asmJitEnvironment() const = 0;
37+
38+
// pthread_atfork() handlers, called via codeAllocatorAtFork*(). Compile
39+
// threads allocate code with the GIL released, so a child forked at the
40+
// wrong moment would otherwise inherit an allocator lock held by a thread
41+
// that no longer exists.
42+
virtual void atForkPrepare() {}
43+
virtual void atForkParent() {}
44+
virtual void atForkChild() {}
3745
};
3846

3947
} // namespace cinderx::jit

cinderx/Jit/compilation_lock.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,31 @@
22

33
#include "cinderx/Jit/compilation_lock.h"
44

5+
#include <new>
6+
57
namespace cinderx::jit {
68

79
std::recursive_mutex& jitCompilationMutex() {
810
static std::recursive_mutex mutex;
911
return mutex;
1012
}
1113

14+
void jitCompilationAtForkPrepare() {
15+
jitCompilationMutex().lock();
16+
}
17+
18+
void jitCompilationAtForkParent() {
19+
jitCompilationMutex().unlock();
20+
}
21+
22+
void jitCompilationAtForkChild() {
23+
// Reuse the storage to get a fresh, unlocked mutex. Unlocking is not an
24+
// option even though the forking thread is the owner: a recursive mutex
25+
// identifies its owner by thread id, and the surviving thread gets a new one
26+
// across the fork, so unlock() would fail with EPERM and leave the lock held
27+
// forever. Destroying it isn't an option either, as it is still locked by
28+
// atForkPrepare(), so its lifetime ends without running its destructor.
29+
new (&jitCompilationMutex()) std::recursive_mutex{};
30+
}
31+
1232
} // namespace cinderx::jit

cinderx/Jit/compilation_lock.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ namespace cinderx::jit {
1212
// logic for GIL handling during threaded compiles.
1313
std::recursive_mutex& jitCompilationMutex();
1414

15+
// pthread_atfork() handlers for the compilation lock. Compile threads hold it
16+
// with the GIL released, so a child forked at the wrong moment would otherwise
17+
// inherit it locked by a thread that no longer exists.
18+
void jitCompilationAtForkPrepare();
19+
void jitCompilationAtForkParent();
20+
void jitCompilationAtForkChild();
21+
1522
// Uses to track if the current thread holds the lock for assertion purposes.
1623
inline thread_local int jitCompilationLockDepth = 0;
1724

0 commit comments

Comments
 (0)