Skip to content

Commit

Permalink
Fix flaky CarbonRouterClient.basicUsageSameThreadClient test
Browse files Browse the repository at this point in the history 
Summary:
CarbonRouterClient.basicUsageSameThreadClient currently does not wait for code running on the proxy EVB to complete, which causes flakiness and errors like the following:

```
*** Aborted at 1734013374 (Unix time, try 'date -d 1734013374') ***
*** Signal 11 (SIGSEGV) (0x0) received by PID 72130 (pthread TID 0x7f4b45a096c0) (linux TID 72132) (code: 128), stack trace: ***
    @ 00000000007f0586 folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/debugging/symbolizer/SignalHandler.cpp:453
    @ 0000000000019dcf (unknown)
    @ 000000000078cdb9 folly::TimeoutManager::clearCobTimeouts()
                       /usr/include/boost/intrusive/detail/list_node.hpp:60
                       -> /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/TimeoutManager.cpp
    @ 000000000078df70 folly::VirtualEventBase::destroyImpl()
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/VirtualEventBase.cpp:38
    @ 00000000007739e6 bool folly::AtomicNotificationQueue<folly::Function<void ()> >::drive<folly::EventBase::FuncRunner&>(folly::EventBase::FuncRunner&)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/Function.h:370
                       -> /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp
    @ 0000000000775183 folly::EventBaseAtomicNotificationQueue<folly::Function<void ()>, folly::EventBase::FuncRunner>::execute()
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBaseAtomicNotificationQueue-inl.h:270
                       -> /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp
    @ 000000000077521c non-virtual thunk to folly::EventBaseAtomicNotificationQueue<folly::Function<void ()>, folly::EventBase::FuncRunner>::handlerReady(unsigned short)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBaseAtomicNotificationQueue-inl.h:279
                       -> /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp
    @ 000000000077a783 folly::EventHandler::libeventCallback(int, short, void*)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventHandler.cpp:159
    @ 00000000000247f9 (unknown)
    @ 000000000002642e event_base_loop
    @ 000000000076e331 folly::EventBase::loopMain(int, folly::EventBase::LoopOptions)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp:99
    @ 000000000076e74d folly::EventBase::loopBody(int, folly::EventBase::LoopOptions)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp:516
    @ 000000000076e7e7 folly::EventBase::loop()
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp:477
    @ 0000000000771336 folly::EventBase::loopForever()
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/io/async/EventBase.cpp:784
    @ 00000000007221eb folly::IOThreadPoolExecutor::threadRun(std::shared_ptr<folly::ThreadPoolExecutor::Thread>)
                       /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/executors/IOThreadPoolExecutor.cpp:241
    @ 000000000072c557 void folly::detail::function::call_<std::_Bind<void (folly::ThreadPoolExecutor::*(folly::ThreadPoolExecutor*, std::shared_ptr<folly::ThreadPoolExecutor::Thread>))(std::shared_ptr<folly::ThreadPoolExecutor::Thread>)>, true, false, void>(, folly::detail::function::Data&)
                       /usr/include/c++/14/bits/invoke.h:74
                       -> /home/mszabo/Documents/fbcode-scratch/repos/git.luolix.top-facebook-folly.git/folly/executors/ThreadPoolExecutor.cpp
    @ 000000000004b523 (unknown)
    @ 0000000000070cd6 start_thread
    @ 00000000000f4c8b __clone3
```

As a fix, add explicit synchronization to ensure the code scheduled on the EVB completes first before attempting to terminate the EVB.

X-link: facebook/mcrouter#458

Reviewed By: disylh

Differential Revision: D67521838

Pulled By: stuclar

fbshipit-source-id: 6f90bacea961a04054f31a9926a4fb2f9958a439
  • Loading branch information
@mszabo-wikia @facebook-github-bot
mszabo-wikia authored and facebook-github-bot committed Dec 20, 2024
1 parent b4ccf64 commit ef48dde
Showing 1 changed file with 19 additions and 14 deletions.
33 changes: 19 additions & 14 deletions third-party/mcrouter/src/mcrouter/test/cpp_unit_tests/McrouterClientUsage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,24 +97,29 @@ TEST(CarbonRouterClient, basicUsageSameThreadClient) {
client->setProxyIndex(0);

bool replyReceived = false;
eventBase.runInEventBaseThread([client = client.get(), &replyReceived]() {
// We must ensure that req will remain alive all the way through the reply
// callback given to client->send(). This demonstrates one way of ensuring
// this.
auto req = std::make_unique<McGetRequest>("key");
auto reqRawPtr = req.get();
client->send(
*reqRawPtr,
[req = std::move(req), &replyReceived](
const McGetRequest&, McGetReply&& reply) {
EXPECT_EQ(carbon::Result::NOTFOUND, *reply.result_ref());
replyReceived = true;
});
});
folly::fibers::Baton baton;

eventBase.runInEventBaseThread(
[client = client.get(), &replyReceived, &baton]() {
// We must ensure that req will remain alive all the way through the
// reply callback given to client->send(). This demonstrates one way of
// ensuring this.
auto req = std::make_unique<McGetRequest>("key");
auto reqRawPtr = req.get();
client->send(
*reqRawPtr,
[req = std::move(req), &replyReceived, &baton](
const McGetRequest&, McGetReply&& reply) {
EXPECT_EQ(carbon::Result::NOTFOUND, *reply.result_ref());
replyReceived = true;
baton.post();
});
});

// Wait for proxy threads to complete outstanding requests and exit
// gracefully. This ensures graceful destruction of the static
// CarbonRouterInstance instance.
baton.wait();
router->shutdown();
ioThreadPool.reset();
EXPECT_TRUE(replyReceived);
Expand Down

0 comments on commit ef48dde

Please sign in to comment.