Skip to content

Commit 6d35781

Browse files
afrindmeta-codesync[bot]
authored andcommitted
Hold a KeepAlive to the main evb
Summary: There's a folly bug that can cause an event posted to the main evb to get stuck unless a KeepAlive is held. When KeepAlive's are held, the internal notification queue is held as a non-internal event. In our case, the main evb was left holding the signal handler, and the event to unregister the handler was never processed. Holding a KeepAlive works around the folly issue. This also consolidates shutdown to a single event posted to the evb, and normalizes some other shutdown paths. Reviewed By: sharmafb Differential Revision: D116505024 fbshipit-source-id: d0882a039603a9b9faf740a8214ac5a3d1ef82e8
1 parent c098dbd commit 6d35781

2 files changed

Lines changed: 12 additions & 13 deletions

File tree

proxygen/lib/http/coro/server/HTTPServer.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ void HTTPServer::start(
140140
}
141141
return;
142142
}
143+
loopKeepAlive_ = folly::getKeepAliveToken(eventBase_);
143144
run(std::move(onSuccess));
144145
// Blocks until all IO Threads have terminated and joined
145146
threadPool.stop();
@@ -355,8 +356,7 @@ void HTTPServer::drain() {
355356
XLOG(DBG4) << __func__;
356357
if (state_ == State::RUNNING) {
357358
state_ = State::DRAINING;
358-
eventBase_.runImmediatelyOrRunInEventBaseThread(
359-
[this] { globalDrainImpl(); });
359+
eventBase_.runImmediatelyOrRunInEventBaseThread([this] { teardownImpl(); });
360360
for (auto& it : acceptors_) {
361361
for (auto& acceptor : it.second) {
362362
if (auto evb = acceptor.getEventBaseKeepalive()) {
@@ -366,11 +366,16 @@ void HTTPServer::drain() {
366366
// drain/forceStop
367367
}
368368
}
369-
eventBase_.runImmediatelyOrRunInEventBaseThread(
370-
[this] { unregisterSignalHandlers(); });
371369
}
372370
}
373371

372+
void HTTPServer::teardownImpl() {
373+
XLOG(DBG4) << __func__;
374+
globalDrainImpl();
375+
unregisterSignalHandlers();
376+
loopKeepAlive_.reset();
377+
}
378+
374379
void HTTPServer::globalDrainImpl() {
375380
XLOG(DBG4) << __func__;
376381
for (const auto& serverSocket : serverSockets_) {
@@ -402,15 +407,7 @@ void HTTPServer::forceStop() {
402407
auto state = state_.load();
403408
if (state == State::RUNNING) {
404409
state_ = state = State::DRAINING;
405-
folly::ExecutorKeepAlive keepAlive(&eventBase_);
406-
eventBase_.runImmediatelyOrRunInEventBaseThread([this] {
407-
for (const auto& serverSocket : serverSockets_) {
408-
XCHECK(serverSocket);
409-
serverSocket->stopAccepting();
410-
}
411-
});
412-
eventBase_.runImmediatelyOrRunInEventBaseThread(
413-
[this] { unregisterSignalHandlers(); });
410+
eventBase_.runImmediatelyOrRunInEventBaseThread([this] { teardownImpl(); });
414411
}
415412
if (state == State::DRAINING) {
416413
if (quicServer_) {

proxygen/lib/http/coro/server/HTTPServer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ class HTTPServer : public quic::QuicHandshakeSocketHolder::Callback {
268268
XLOG(ERR) << "Failed to accept QUIC connection: " << error.message;
269269
}
270270
void run(std::function<void()> onSuccess);
271+
void teardownImpl();
271272
void globalDrainImpl();
272273
void unregisterSignalHandlers();
273274
void drainImpl(HTTPCoroAcceptor& acceptor);
@@ -276,6 +277,7 @@ class HTTPServer : public quic::QuicHandshakeSocketHolder::Callback {
276277
std::shared_ptr<HTTPHandler> handler_;
277278
folly::F14FastSet<Observer*> observers_;
278279
folly::EventBase eventBase_;
280+
folly::Executor::KeepAlive<folly::EventBase> loopKeepAlive_;
279281
std::vector<folly::AsyncServerSocket::UniquePtr> serverSockets_;
280282
bool setReusePortSocketOption_{false};
281283
folly::F14NodeMap<folly::EventBase*, std::list<HTTPCoroAcceptor>> acceptors_;

0 commit comments

Comments
 (0)