Skip to content

Commit b6cf382

Browse files
agrawaldeveshmeta-codesync[bot]
authored andcommitted
Fix ThreadCachedArena zombies_ alignment mismatch
Summary: Fixing the crash seen during shutdown: P2279276864 ThreadCachedArena stores maxAlign_ and passes it to per-thread arenas via allocateThreadLocalArena(), but the zombies_ arena was constructed with only minBlockSize — using the default maxAlign. When maxAlign_ != kDefaultMaxAlign, the two arenas have different blockGoodAllocSize() values, and Arena::merge asserts on thread exit during zombify(). Fix: pass maxAlign to the zombies_ arena constructor so it matches per-thread arenas. This is a latent bug that surfaces whenever ThreadCachedArena is constructed with non-default alignment, e.g. from Unicorn's NodeArena. Reviewed By: luciang Differential Revision: D101491707 fbshipit-source-id: 90691bf83ec2f79e889bf8ac7ddbea482041fdc6
1 parent e7786cf commit b6cf382

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

folly/memory/ThreadCachedArena.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace folly {
2323
ThreadCachedArena::ThreadCachedArena(size_t minBlockSize, size_t maxAlign)
2424
: minBlockSize_(minBlockSize),
2525
maxAlign_(maxAlign),
26-
zombies_(std::in_place, minBlockSize) {}
26+
zombies_(std::in_place, minBlockSize, SysArena::kNoSizeLimit, maxAlign) {}
2727

2828
SysArena* ThreadCachedArena::allocateThreadLocalArena() {
2929
auto arena = new SysArena(minBlockSize_, SysArena::kNoSizeLimit, maxAlign_);

folly/memory/test/ThreadCachedArenaTest.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,35 @@ TEST(ThreadCachedArena, MultiThreaded) {
151151
mainTester.verify();
152152
}
153153

154+
TEST(ThreadCachedArena, MultiThreadedCustomAlign) {
155+
static const size_t requestedBlockSize = 64;
156+
// Use a larger-than-default alignment to exercise the maxAlign path.
157+
// This triggers Arena::merge between zombies_ (constructed at arena init)
158+
// and per-thread arenas (constructed on first allocation per thread).
159+
static const size_t customAlign = 64;
160+
ThreadCachedArena arena(requestedBlockSize, customAlign);
161+
ArenaTester mainTester(arena);
162+
163+
static const size_t numThreads = 20;
164+
for (size_t i = 0; i < 2; i++) {
165+
std::vector<std::thread> threads;
166+
threads.reserve(numThreads);
167+
for (size_t j = 0; j < numThreads; j++) {
168+
threads.emplace_back([&arena, &mainTester]() {
169+
ArenaTester tester(arena);
170+
tester.allocate(500, 1 << 10);
171+
tester.verify();
172+
mainTester.merge(std::move(tester));
173+
});
174+
}
175+
for (auto& t : threads) {
176+
t.join();
177+
}
178+
}
179+
180+
mainTester.verify();
181+
}
182+
154183
TEST(ThreadCachedArena, ThreadCachedArenaAllocator) {
155184
using Map = std::unordered_map<
156185
int,

0 commit comments

Comments
 (0)