Skip to content

Commit 5ad8dce

Browse files
ktosoktoso
committed
Revert "Wipe packet buffers that held serialized WipedString (apple#10018)"
This reverts commit e2df6e3.
1 parent ebbc238 commit 5ad8dce

File tree

11 files changed

+101
-505
lines changed

11 files changed

+101
-505
lines changed

‎fdbserver/workloads/UnitTests.actor.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ void forceLinkArenaStringTests();
4949
void forceLinkActorCollectionTests();
5050
void forceLinkDDSketchTests();
5151
void forceLinkCommitProxyTests();
52-
void forceLinkWipedStringTests();
5352

5453
struct UnitTestWorkload : TestWorkload {
5554
static constexpr auto NAME = "UnitTests";
@@ -114,7 +113,6 @@ struct UnitTestWorkload : TestWorkload {
114113
forceLinkArenaStringTests();
115114
forceLinkActorCollectionTests();
116115
forceLinkDDSketchTests();
117-
forceLinkWipedStringTests();
118116
}
119117

120118
Future<Void> setup(Database const& cx) override {

‎flow/Arena.cpp‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,22 @@ void* ArenaBlock::allocate(Reference<ArenaBlock>& self, int bytes, IsSecureMem i
344344
return result;
345345
}
346346

347+
force_inline ArenaBlock* newBigArenaBlock(int size) {
348+
if (keepalive_allocator::isActive()) [[unlikely]] {
349+
return (ArenaBlock*)keepalive_allocator::allocate(size);
350+
} else {
351+
return (ArenaBlock*)new uint8_t[size];
352+
}
353+
}
354+
355+
force_inline void deleteBigArenaBlock(ArenaBlock* block) {
356+
if (keepalive_allocator::isActive()) [[unlikely]] {
357+
return keepalive_allocator::invalidate(block);
358+
} else {
359+
return delete[] reinterpret_cast<uint8_t*>(block);
360+
}
361+
}
362+
347363
// Return an appropriately-sized ArenaBlock to store the given data
348364
ArenaBlock* ArenaBlock::create(int dataSize, Reference<ArenaBlock>& next) {
349365
ArenaBlock* b;
@@ -386,23 +402,23 @@ ArenaBlock* ArenaBlock::create(int dataSize, Reference<ArenaBlock>& next) {
386402
b->bigSize = 256;
387403
INSTRUMENT_ALLOCATE("Arena256");
388404
} else if (reqSize <= 512) {
389-
b = (ArenaBlock*)allocateAndMaybeKeepalive(512);
405+
b = newBigArenaBlock(512);
390406
b->bigSize = 512;
391407
INSTRUMENT_ALLOCATE("Arena512");
392408
} else if (reqSize <= 1024) {
393-
b = (ArenaBlock*)allocateAndMaybeKeepalive(1024);
409+
b = newBigArenaBlock(1024);
394410
b->bigSize = 1024;
395411
INSTRUMENT_ALLOCATE("Arena1024");
396412
} else if (reqSize <= 2048) {
397-
b = (ArenaBlock*)allocateAndMaybeKeepalive(2048);
413+
b = newBigArenaBlock(2048);
398414
b->bigSize = 2048;
399415
INSTRUMENT_ALLOCATE("Arena2048");
400416
} else if (reqSize <= 4096) {
401-
b = (ArenaBlock*)allocateAndMaybeKeepalive(4096);
417+
b = newBigArenaBlock(4096);
402418
b->bigSize = 4096;
403419
INSTRUMENT_ALLOCATE("Arena4096");
404420
} else {
405-
b = (ArenaBlock*)allocateAndMaybeKeepalive(8192);
421+
b = newBigArenaBlock(8192);
406422
b->bigSize = 8192;
407423
INSTRUMENT_ALLOCATE("Arena8192");
408424
}
@@ -414,7 +430,7 @@ ArenaBlock* ArenaBlock::create(int dataSize, Reference<ArenaBlock>& next) {
414430
#ifdef ALLOC_INSTRUMENTATION
415431
allocInstr["ArenaHugeKB"].alloc((reqSize + 1023) >> 10);
416432
#endif
417-
b = (ArenaBlock*)allocateAndMaybeKeepalive(reqSize);
433+
b = newBigArenaBlock(reqSize);
418434
b->tinySize = b->tinyUsed = NOT_TINY;
419435
b->bigSize = reqSize;
420436
b->totalSizeEstimate = b->bigSize;
@@ -506,26 +522,26 @@ void ArenaBlock::destroyLeaf() {
506522
FastAllocator<256>::release(this);
507523
INSTRUMENT_RELEASE("Arena256");
508524
} else if (bigSize <= 512) {
509-
freeOrMaybeKeepalive(this);
525+
deleteBigArenaBlock(this);
510526
INSTRUMENT_RELEASE("Arena512");
511527
} else if (bigSize <= 1024) {
512-
freeOrMaybeKeepalive(this);
528+
deleteBigArenaBlock(this);
513529
INSTRUMENT_RELEASE("Arena1024");
514530
} else if (bigSize <= 2048) {
515-
freeOrMaybeKeepalive(this);
531+
deleteBigArenaBlock(this);
516532
INSTRUMENT_RELEASE("Arena2048");
517533
} else if (bigSize <= 4096) {
518-
freeOrMaybeKeepalive(this);
534+
deleteBigArenaBlock(this);
519535
INSTRUMENT_RELEASE("Arena4096");
520536
} else if (bigSize <= 8192) {
521-
freeOrMaybeKeepalive(this);
537+
deleteBigArenaBlock(this);
522538
INSTRUMENT_RELEASE("Arena8192");
523539
} else {
524540
#ifdef ALLOC_INSTRUMENTATION
525541
allocInstr["ArenaHugeKB"].dealloc((bigSize + 1023) >> 10);
526542
#endif
527543
g_hugeArenaMemory.fetch_sub(bigSize);
528-
freeOrMaybeKeepalive(this);
544+
deleteBigArenaBlock(this);
529545
}
530546
}
531547
}

‎flow/FastAlloc.cpp‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,6 @@ namespace detail {
310310

311311
std::set<void*> g_allocatedSet;
312312
std::set<void*> g_freedSet;
313-
std::vector<std::pair<const uint8_t*, int>> g_wipedSet;
314313
bool g_active = false;
315314

316315
} // namespace detail
@@ -320,52 +319,33 @@ ActiveScope::ActiveScope() {
320319
ASSERT(!detail::g_active);
321320
ASSERT(detail::g_allocatedSet.empty());
322321
ASSERT(detail::g_freedSet.empty());
323-
ASSERT(detail::g_wipedSet.empty());
324322
detail::g_active = true;
325-
// As of writing, TraceEvent uses eventname-based throttling keyed by Standalone<StringRef>,
326-
// which uses Arena and stays allocated after scope.
327-
// Therefore, we disable allocation tracing (e.g. hugeArenaSample()) while this scope is active.
328-
g_allocation_tracing_disabled++;
329323
}
330324

331325
ActiveScope::~ActiveScope() {
332326
ASSERT_ABORT(detail::g_active);
333327
ASSERT_ABORT(detail::g_allocatedSet == detail::g_freedSet);
334-
g_allocation_tracing_disabled--;
335328
for (auto memory : detail::g_allocatedSet) {
336-
delete[] static_cast<uint8_t*>(memory);
329+
delete[] reinterpret_cast<uint8_t*>(memory);
337330
}
338331
detail::g_allocatedSet.clear();
339332
detail::g_freedSet.clear();
340-
detail::g_wipedSet.clear();
341333
detail::g_active = false;
342334
}
343335

344336
void* allocate(size_t size) {
345-
ASSERT_ABORT(detail::g_active);
346337
auto ptr = new uint8_t[size];
347338
auto [_, inserted] = detail::g_allocatedSet.insert(ptr);
348-
ASSERT_ABORT(inserted); // no duplicates
339+
ASSERT(inserted); // no duplicates
349340
return ptr;
350341
}
351342

352343
void invalidate(void* ptr) {
353-
ASSERT_ABORT(detail::g_active);
354-
ASSERT_ABORT(detail::g_allocatedSet.contains(ptr));
355-
ASSERT_ABORT(!detail::g_freedSet.contains(ptr));
344+
ASSERT(detail::g_allocatedSet.contains(ptr));
345+
ASSERT(!detail::g_freedSet.contains(ptr));
356346
detail::g_freedSet.insert(ptr);
357347
}
358348

359-
void trackWipedArea(const uint8_t* begin, int size) {
360-
ASSERT_ABORT(detail::g_active);
361-
detail::g_wipedSet.emplace_back(begin, size);
362-
}
363-
364-
std::vector<std::pair<const uint8_t*, int>> const& getWipedAreaSet() {
365-
ASSERT_ABORT(detail::g_active);
366-
return detail::g_wipedSet;
367-
}
368-
369349
} // namespace keepalive_allocator
370350

371351
template <int Size>

‎flow/Knobs.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ void FlowKnobs::initialize(Randomize randomize, IsSimulated isSimulated) {
145145
init( PUBLIC_KEY_FILE_REFRESH_INTERVAL_SECONDS, 300 );
146146
init( AUDIT_TIME_WINDOW, 5.0 );
147147
init( TOKEN_CACHE_SIZE, 2000 );
148-
init( WIPE_SENSITIVE_DATA_FROM_PACKET_BUFFER, true );
149148

150149
//AsyncFileCached
151150
init( PAGE_CACHE_4K, 2LL<<30 );

0 commit comments

Comments
 (0)