Convert Callback class to singly linked list #1

sfc-gh-ljoswiak · 2021-12-14T01:24:51Z

Initial attempt at converting the Callback struct from a doubly linked list to a singly linked list. Initial benchmarks show slightly worse performance with this change.

merge

Previously we did not null out "attributes" in `Span& Span::operator=(Span&& o);`, but we destroyed the arena owning memory referenced by "attributes". Fix that by nulling out "attributes", and rewrite it in a way that's (hopefully) less error-prone. ASAN diagnostic: ``` ==24==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ================================================================= ==24==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300033fa06 at pc 0x0000026eadcf bp 0x7ffca646fe50 sp 0x7ffca646f618 READ of size 9 at 0x60300033fa06 thread T0 #0 0x26eadce in __asan_memmove /tmp/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:30:3 #1 0xd43db76 in __copy<const unsigned char, unsigned char> /usr/local/bin/../include/c++/v1/__algorithm/copy.h:59:9 #2 0xd43db76 in copy<const unsigned char *, unsigned char *> /usr/local/bin/../include/c++/v1/__algorithm/copy.h:72:13 #3 0xd43db76 in write_bytes /mnt/ephemeral/anoyes/foundationdb/fdbrpc/include/fdbrpc/Msgpack.h:45:3 #4 0xd43db76 in serialize_string(unsigned char const*, int, MsgpackBuffer&) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/include/fdbrpc/Msgpack.h:109:6 apple#5 0xd43d208 in void serialize_map<SmallVectorRef<KeyValueRef, 1> >(SmallVectorRef<KeyValueRef, 1> const&, MsgpackBuffer&) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/include/fdbrpc/Msgpack.h:154:3 apple#6 0xd42e2a2 in (anonymous namespace)::UDPTracer::serialize_span(Span const&, MsgpackBuffer&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/Tracing.actor.cpp:157:3 apple#7 0xd42c8f4 in (anonymous namespace)::FastUDPTracer::trace(Span const&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/Tracing.actor.cpp:301:3 apple#8 0xd41dfe2 in Span::operator=(Span&&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/Tracing.actor.cpp:355:13 apple#9 0xc3eb122 in (anonymous namespace)::ReadVersionBatcherActorState<(anonymous namespace)::ReadVersionBatcherActor>::a_body1loopBody1cont1(int) /mnt/ephemeral/anoyes/foundationdb/fdbclient/NativeAPI.actor.cpp:7180:9 apple#10 0xc3e386d in a_body1loopBody1when2 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbclient/NativeAPI.actor.g.cpp:32322:15 apple#11 0xc3e386d in a_callback_fire /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbclient/NativeAPI.actor.g.cpp:32434:4 apple#12 0xc3e386d in ActorCallback<(anonymous namespace)::ReadVersionBatcherActor, 1, Void>::fire(Void const&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:1316:34 apple#13 0x278af3f in void SAV<Void>::send<Void>(Void&&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:655:23 apple#14 0xdd2dc36 in send<Void> /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:901:8 apple#15 0xdd2dc36 in Sim2::execTask(Sim2::PromiseTask&) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/sim2.actor.cpp:2319:15 apple#16 0xdd2cf0e in Sim2::runLoop(Sim2*) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/sim2.actor.cpp:1279:11 apple#17 0x7ebdf2e in main /mnt/ephemeral/anoyes/foundationdb/fdbserver/fdbserver.actor.cpp:2276:17 apple#18 0x7f62c7be6554 in __libc_start_main (/lib64/libc.so.6+0x22554) apple#19 0x266f028 in _start (/mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/bin/fdbserver+0x266f028) 0x60300033fa06 is located 6 bytes inside of 32-byte region [0x60300033fa00,0x60300033fa20) freed by thread T0 here: #0 0x26eb352 in free /tmp/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:111:3 #1 0xdd90118 in ArenaBlock::destroy() /mnt/ephemeral/anoyes/foundationdb/flow/Arena.cpp:466:6 #2 0xdd8f0b0 in delref /mnt/ephemeral/anoyes/foundationdb/flow/Arena.cpp:173:3 #3 0xdd8f0b0 in delref<ArenaBlock> /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/FastRef.h:95:7 #4 0xdd8f0b0 in operator= /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/FastRef.h:147:5 apple#5 0xdd8f0b0 in Arena::operator=(Arena&&) /mnt/ephemeral/anoyes/foundationdb/flow/Arena.cpp:119:36 apple#6 0xd41dfed in Span::operator=(Span&&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/Tracing.actor.cpp:357:8 apple#7 0xc3eb122 in (anonymous namespace)::ReadVersionBatcherActorState<(anonymous namespace)::ReadVersionBatcherActor>::a_body1loopBody1cont1(int) /mnt/ephemeral/anoyes/foundationdb/fdbclient/NativeAPI.actor.cpp:7180:9 apple#8 0xc3e386d in a_body1loopBody1when2 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbclient/NativeAPI.actor.g.cpp:32322:15 apple#9 0xc3e386d in a_callback_fire /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbclient/NativeAPI.actor.g.cpp:32434:4 apple#10 0xc3e386d in ActorCallback<(anonymous namespace)::ReadVersionBatcherActor, 1, Void>::fire(Void const&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:1316:34 apple#11 0x278af3f in void SAV<Void>::send<Void>(Void&&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:655:23 apple#12 0xdd2dc36 in send<Void> /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:901:8 apple#13 0xdd2dc36 in Sim2::execTask(Sim2::PromiseTask&) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/sim2.actor.cpp:2319:15 apple#14 0xdd2cf0e in Sim2::runLoop(Sim2*) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/sim2.actor.cpp:1279:11 apple#15 0x7ebdf2e in main /mnt/ephemeral/anoyes/foundationdb/fdbserver/fdbserver.actor.cpp:2276:17 apple#16 0x7f62c7be6554 in __libc_start_main (/lib64/libc.so.6+0x22554) previously allocated by thread T0 here: #0 0x26ebeb2 in aligned_alloc /tmp/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:176:3 #1 0xdd8e5e0 in ArenaBlock::create(int, Reference<ArenaBlock>&) /mnt/ephemeral/anoyes/foundationdb/flow/Arena.cpp:339:21 #2 0xdd91139 in ArenaBlock::allocate(Reference<ArenaBlock>&, int) /mnt/ephemeral/anoyes/foundationdb/flow/Arena.cpp:322:7 #3 0x2bbf49b in operator new[] /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/Arena.h:206:9 #4 0x2bbf49b in StringRef /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/Arena.h:446:54 apple#5 0x2bbf49b in Span::Span(SpanContext const&, Location const&, SpanContext const&, std::initializer_list<SpanContext> const&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/include/fdbclient/Tracing.h:141:33 apple#6 0x2bbef25 in Span::Span(Location const&, SpanContext const&, std::initializer_list<SpanContext> const&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/include/fdbclient/Tracing.h:148:6 apple#7 0xc1bd941 in Span /mnt/ephemeral/anoyes/foundationdb/fdbclient/include/fdbclient/Tracing.h:160:44 apple#8 0xc1bd941 in ReadVersionBatcherActorState /mnt/ephemeral/anoyes/foundationdb/fdbclient/NativeAPI.actor.cpp:7125:6 apple#9 0xc1bd941 in ReadVersionBatcherActor /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbclient/NativeAPI.actor.g.cpp:32621:6 apple#10 0xc1bd941 in readVersionBatcher(DatabaseContext* const&, FutureStream<DatabaseContext::VersionRequest> const&, TransactionPriority const&, unsigned int const&) /mnt/ephemeral/anoyes/foundationdb/fdbclient/NativeAPI.actor.cpp:7098:26 apple#11 0xc1b316a in Transaction::getReadVersion(unsigned int) /mnt/ephemeral/anoyes/foundationdb/fdbclient/NativeAPI.actor.cpp:7366:8 apple#12 0xc19055f in getReadVersion /mnt/ephemeral/anoyes/foundationdb/fdbclient/include/fdbclient/NativeAPI.actor.h:315:44 apple#13 0xc19055f in Transaction::get(Standalone<StringRef> const&, Snapshot) /mnt/ephemeral/anoyes/foundationdb/fdbclient/NativeAPI.actor.cpp:5314:13 apple#14 0x5eca2f1 in a_body1 /mnt/ephemeral/anoyes/foundationdb/fdbserver/MoveKeys.actor.cpp:190:55 apple#15 0x5eca2f1 in ReadMoveKeysLockActor /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/MoveKeys.actor.g.cpp:1001:9 apple#16 0x5eca2f1 in readMoveKeysLock(Transaction* const&, MoveKeysLock* const&) /mnt/ephemeral/anoyes/foundationdb/fdbserver/MoveKeys.actor.cpp:188:26 apple#17 0x5ee590d in a_body1loopBody1 /mnt/ephemeral/anoyes/foundationdb/fdbserver/MoveKeys.actor.cpp:228:39 apple#18 0x5ee590d in (anonymous namespace)::TakeMoveKeysLockActorState<(anonymous namespace)::TakeMoveKeysLockActor>::a_body1loopHead1(int) /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/MoveKeys.actor.g.cpp:1421:49 apple#19 0x5ecb801 in a_body1 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/MoveKeys.actor.g.cpp:1400:16 apple#20 0x5ecb801 in TakeMoveKeysLockActor /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/MoveKeys.actor.g.cpp:1807:9 apple#21 0x5ecb801 in takeMoveKeysLock(Database const&, UID const&) /mnt/ephemeral/anoyes/foundationdb/fdbserver/MoveKeys.actor.cpp:216:34 apple#22 0x542a421 in DDTxnProcessor::takeMoveKeysLock(UID const&) const /mnt/ephemeral/anoyes/foundationdb/fdbserver/DDTxnProcessor.actor.cpp:620:9 apple#23 0x562aa75 in takeMoveKeysLock /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:330:69 apple#24 0x562aa75 in DataDistributor::InitActorState<DataDistributor::InitActor>::a_body1loopBody1(int) /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:370:44 apple#25 0x562a0ba in a_body1loopHead1 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:1116:49 apple#26 0x562a0ba in DataDistributor::InitActorState<DataDistributor::InitActor>::a_body1(int) /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:1083:16 apple#27 0x55088c0 in InitActor /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:1715:9 apple#28 0x55088c0 in init /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:367:26 apple#29 0x55088c0 in a_body1loopBody1 /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:611:39 apple#30 0x55088c0 in (anonymous namespace)::DataDistributionActorState<(anonymous namespace)::DataDistributionActor>::a_body1loopHead1(int) /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:2772:49 apple#31 0x54e9cf1 in a_body1 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:2751:16 apple#32 0x54e9cf1 in DataDistributionActor /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:3976:9 apple#33 0x54e9cf1 in dataDistribution(Reference<DataDistributor> const&, PromiseStream<GetMetricsListRequest> const&) /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:586:26 apple#34 0x54f8758 in a_body1 /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:1533:39 apple#35 0x54f8758 in DataDistributorActor /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/DataDistribution.actor.g.cpp:11473:9 apple#36 0x54f8758 in dataDistributor(DataDistributorInterface const&, Reference<AsyncVar<ServerDBInfo> const> const&) /mnt/ephemeral/anoyes/foundationdb/fdbserver/DataDistribution.actor.cpp:1517:26 apple#37 0x88f3f88 in (anonymous namespace)::WorkerServerActorState<(anonymous namespace)::WorkerServerActor>::a_body1cont10loopBody1when6(InitializeDataDistributorRequest&&, int) /mnt/ephemeral/anoyes/foundationdb/fdbserver/worker.actor.cpp:2219:42 apple#38 0x8871c15 in a_callback_fire /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbserver/worker.actor.g.cpp:13169:4 apple#39 0x8871c15 in ActorSingleCallback<(anonymous namespace)::WorkerServerActor, 6, InitializeDataDistributorRequest>::fire(InitializeDataDistributorRequest&&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:1338:34 apple#40 0x33e13c0 in send<InitializeDataDistributorRequest> /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:1004:29 apple#41 0x33e13c0 in NetNotifiedQueue<InitializeDataDistributorRequest, false>::receive(ArenaObjectReader&) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/include/fdbrpc/fdbrpc.h:702:10 apple#42 0xd91b6c6 in (anonymous namespace)::DeliverActorState<(anonymous namespace)::DeliverActor>::a_body1cont1(int) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/FlowTransport.actor.cpp:1042:15 apple#43 0xd91a37d in a_body1cont2 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbrpc/FlowTransport.actor.g.cpp:4333:15 apple#44 0xd91a37d in a_body1when1 /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbrpc/FlowTransport.actor.g.cpp:4345:15 apple#45 0xd91a37d in a_callback_fire /mnt/ephemeral/anoyes/build/foundationdb.linux.clang.asan.x86_64/fdbrpc/FlowTransport.actor.g.cpp:4366:4 apple#46 0xd91a37d in ActorCallback<(anonymous namespace)::DeliverActor, 0, Void>::fire(Void const&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:1316:34 apple#47 0x278af3f in void SAV<Void>::send<Void>(Void&&) /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:655:23 apple#48 0xdd2dc36 in send<Void> /mnt/ephemeral/anoyes/foundationdb/flow/include/flow/flow.h:901:8 apple#49 0xdd2dc36 in Sim2::execTask(Sim2::PromiseTask&) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/sim2.actor.cpp:2319:15 apple#50 0xdd2cf0e in Sim2::runLoop(Sim2*) /mnt/ephemeral/anoyes/foundationdb/fdbrpc/sim2.actor.cpp:1279:11 apple#51 0x7ebdf2e in main /mnt/ephemeral/anoyes/foundationdb/fdbserver/fdbserver.actor.cpp:2276:17 apple#52 0x7f62c7be6554 in __libc_start_main (/lib64/libc.so.6+0x22554) SUMMARY: AddressSanitizer: heap-use-after-free /tmp/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:30:3 in __asan_memmove Shadow bytes around the buggy address: 0x0c068005fef0: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd 0x0c068005ff00: fd fd fa fa fd fd fd fd fa fa fd fd fd fa fa fa 0x0c068005ff10: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd 0x0c068005ff20: fa fa fd fd fd fa fa fa fd fd fd fd fa fa fd fd 0x0c068005ff30: fd fd fa fa fd fd fd fd fa fa fd fd fd fa fa fa =>0x0c068005ff40:[fd]fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd 0x0c068005ff50: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd 0x0c068005ff60: fd fa fa fa fd fd fd fa fa fa fd fd fd fd fa fa 0x0c068005ff70: fd fd fd fd fa fa fd fd fd fa fa fa 00 00 00 00 0x0c068005ff80: fa fa fd fd fd fd fa fa 00 00 00 00 fa fa fd fd 0x0c068005ff90: fd fd fa fa 00 00 00 00 fa fa 00 00 00 00 fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ```

* EaR - Misc fixes found using end-to-end integration testing Description Major changes proposed includes: 1. RESTClient filtering of trailing `/`(s) characters from input URI resource path 2. Avoid EKP exponential backup given RESTClient supports exponential backoffs retries for all retryable errors. 3. Memory allocation optimizations: 3.1. BaseCipher key management using Standalone semantics in KMSConnector interface endpoints 3.2. Optimize memcpy while looking encryption-keys in EKP endpoints 4. Avoid delay while starting EKP, given its criticality during cluster recovery. 5. Update BlobCipher to handle variable size BaseCipher buffer 6. Improved logging Testing Setup: 1. External KMS server to supply encryption keys (inhouse) 2. Create cluster with: cluster_aware & domain_aware config * Fix EncryptionOps test Description Testing * EaR - Misc fixes found using end-to-end integration testing Description Major changes: 1. Cleanup EKP driven exponential backup files. 2. Update EKP not to use #1. Testing * EaR - Misc fixes found using end-to-end integration testing Description Address review comments Testing * Fix AES 256 key length value Description Testing * Address review comments Description Testing

…newline char Description The test does the following: 1.Randomly appends new-line character to the token value buffer 2. If #1 is done, it generates temp token file with buffer containing new-line character. 3. Also, it remember the original token for future validation 4. The code parse and read token-validation files and removes the new-line character as desired. Failure in this case due to random buffer used to populate token value contained newline character which was used for validation, however, the file parse/read code as expected removed the newline character, hence causing the mismatch. Patch addresses the concern by ensuring test generated random token-value has no trailing newline chars. Testing tests/fast/RandomUnitTests.toml -s 1355028229

…) (apple#9859) * EaR - Misc fixes found using end-to-end integration testing Description Major changes proposed includes: 1. RESTClient filtering of trailing `/`(s) characters from input URI resource path 2. Avoid EKP exponential backup given RESTClient supports exponential backoffs retries for all retryable errors. 3. Memory allocation optimizations: 3.1. BaseCipher key management using Standalone semantics in KMSConnector interface endpoints 3.2. Optimize memcpy while looking encryption-keys in EKP endpoints 4. Avoid delay while starting EKP, given its criticality during cluster recovery. 5. Update BlobCipher to handle variable size BaseCipher buffer 6. Improved logging Testing Setup: 1. External KMS server to supply encryption keys (inhouse) 2. Create cluster with: cluster_aware & domain_aware config * Fix EncryptionOps test Description Testing * EaR - Misc fixes found using end-to-end integration testing Description Major changes: 1. Cleanup EKP driven exponential backup files. 2. Update EKP not to use #1. Testing * EaR - Misc fixes found using end-to-end integration testing Description Address review comments Testing * Fix AES 256 key length value Description Testing * Address review comments Description Testing (cherry picked from commit 3f6fcad) (apple#9806)

…newline char (apple#9951) Description The test does the following: 1.Randomly appends new-line character to the token value buffer 2. If #1 is done, it generates temp token file with buffer containing new-line character. 3. Also, it remember the original token for future validation 4. The code parse and read token-validation files and removes the new-line character as desired. Failure in this case due to random buffer used to populate token value contained newline character which was used for validation, however, the file parse/read code as expected removed the newline character, hence causing the mismatch. Patch addresses the concern by ensuring test generated random token-value has no trailing newline chars. Testing tests/fast/RandomUnitTests.toml -s 1355028229 (cherry picked from commit 8e52082)

sfc-gh-ljoswiak added 4 commits December 13, 2021 17:22

Modify Callback class from doubly to singly linked list

e617844

Modify remove logic

03b5d87

Bugfix

f77660a

More bugfixes

0efbb96

sfc-gh-tclinkenbeard pushed a commit that referenced this pull request Aug 2, 2022

Merge pull request #1 from apple/main

c0e13b8

merge

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert Callback class to singly linked list #1

Convert Callback class to singly linked list #1

sfc-gh-ljoswiak commented Dec 14, 2021

Convert Callback class to singly linked list #1

Are you sure you want to change the base?

Convert Callback class to singly linked list #1

Conversation

sfc-gh-ljoswiak commented Dec 14, 2021