mirror of
https://github.com/facebook/sapling.git
synced 2024-10-08 07:49:11 +03:00
fix hangs in FuseChannel shutdown
Summary: The FuseChannel unit tests would sometimes hang because a worker thread would still be stuck in a blocking read() call. The issue appears to be that the kernel would often automatically restart the interrupted read(), so the worker would never wake up. This switches the code from using SIGPIPE to wake up, and instead uses SIGUSR2, and installs a handler for SIGUSR2 with the SA_RESTART flag unset. It's possible that this was an issue only for the unit tests, where the FUSE device is a socket. I'm not sure if the kernel would automatically restart read operations on a real FUSE device. There are still theoretically some race conditions here, since `requestSessionExit()` could be called right after a worker thread checks `(runState_` and before it enters the blocking `read()` call. Fixing that would likely require switching to something like `folly::EventBase` or manually using epoll. Fortunately I haven't seen that be an issue in practice so far. Reviewed By: chadaustin, wez Differential Revision: D7282002 fbshipit-source-id: 99d29de13a7858dd25f258fdc94d8f8c71ee84d9
This commit is contained in:
parent
fdecf19d91
commit
6cc56a6f81
@ -131,6 +131,31 @@ StringPiece fuseOpcodeName(FuseOpcode opcode) {
|
||||
using Handler = folly::Future<folly::Unit> (
|
||||
FuseChannel::*)(const fuse_in_header* header, const uint8_t* arg);
|
||||
|
||||
void sigusr2Handler(int /* signum */) {
|
||||
// Do nothing.
|
||||
// The purpose of this signal is only to interrupt the blocking read() calls
|
||||
// in processSession() and readInitPacket()
|
||||
}
|
||||
|
||||
void installSignalHandler() {
|
||||
// We use SIGUSR2 to wake up our worker threads when we want to shut down.
|
||||
// Install a signal handler for this signal. The signal handler itself is a
|
||||
// no-op, we simply want to use it to interrupt blocking read() calls.
|
||||
//
|
||||
// We will re-install this handler each time a FuseChannel object is called,
|
||||
// but that should be fine.
|
||||
//
|
||||
// This must be installed using sigaction() rather than signal(), so we can
|
||||
// ensure that the SA_RESTART flag is not ste.
|
||||
struct sigaction action = {};
|
||||
action.sa_handler = sigusr2Handler;
|
||||
sigemptyset(&action.sa_mask);
|
||||
action.sa_flags = 0; // We intentionally turn off SA_RESTART
|
||||
struct sigaction oldAction;
|
||||
folly::checkUnixError(
|
||||
sigaction(SIGUSR2, &action, &oldAction), "failed to set SIGUSR2 handler");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
struct FuseChannel::HandlerEntry {
|
||||
@ -320,6 +345,7 @@ FuseChannel::FuseChannel(
|
||||
mountPath_(mountPath),
|
||||
fuseDevice_(std::move(fuseDevice)) {
|
||||
CHECK_GE(numThreads_, 1);
|
||||
installSignalHandler();
|
||||
}
|
||||
|
||||
folly::Future<folly::Unit> FuseChannel::initialize() {
|
||||
@ -501,16 +527,31 @@ void FuseChannel::requestSessionExit(
|
||||
std::memory_order_relaxed,
|
||||
std::memory_order_relaxed)) {
|
||||
// Knock our workers out of their blocking read() syscalls
|
||||
// TODO: This code is slightly racy, since threads could receive the signal
|
||||
// immediately before entering read()
|
||||
for (auto& thr : state->workerThreads) {
|
||||
if (thr.joinable() && thr.get_id() != std::this_thread::get_id()) {
|
||||
pthread_kill(thr.native_handle(), SIGPIPE);
|
||||
pthread_kill(thr.native_handle(), SIGUSR2);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void FuseChannel::setThreadSigmask() {
|
||||
// Make sure our thread will receive SIGUSR2
|
||||
sigset_t sigset;
|
||||
sigemptyset(&sigset);
|
||||
sigaddset(&sigset, SIGUSR2);
|
||||
|
||||
sigset_t oldset;
|
||||
sigemptyset(&oldset);
|
||||
|
||||
folly::checkPosixError(pthread_sigmask(SIG_UNBLOCK, &sigset, &oldset));
|
||||
}
|
||||
|
||||
void FuseChannel::initWorkerThread() noexcept {
|
||||
try {
|
||||
setThreadSigmask();
|
||||
setThreadName(to<std::string>("fuse", mountPath_.basename()));
|
||||
|
||||
// Read the INIT packet
|
||||
@ -536,6 +577,7 @@ void FuseChannel::initWorkerThread() noexcept {
|
||||
|
||||
void FuseChannel::fuseWorkerThread() noexcept {
|
||||
setThreadName(to<std::string>("fuse", mountPath_.basename()));
|
||||
setThreadSigmask();
|
||||
|
||||
try {
|
||||
processSession();
|
||||
|
@ -358,6 +358,7 @@ class FuseChannel {
|
||||
const fuse_in_header* header,
|
||||
const uint8_t* arg);
|
||||
|
||||
void setThreadSigmask();
|
||||
void initWorkerThread() noexcept;
|
||||
void fuseWorkerThread() noexcept;
|
||||
void maybeDispatchSessionComplete();
|
||||
|
@ -18,8 +18,5 @@ int main(int argc, char* argv[]) {
|
||||
folly::init(&argc, &argv);
|
||||
folly::initLogging(FLAGS_logging);
|
||||
|
||||
// The FuseChannel code sends SIGPIPE and expects it to be ignored.
|
||||
::signal(SIGPIPE, SIG_IGN);
|
||||
|
||||
return RUN_ALL_TESTS();
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user