diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -69,6 +69,7 @@ Selection.cpp SemanticHighlighting.cpp SemanticSelection.cpp + Shutdown.cpp SourceCode.cpp QueryDriverDatabase.cpp Threading.cpp diff --git a/clang-tools-extra/clangd/JSONTransport.cpp b/clang-tools-extra/clangd/JSONTransport.cpp --- a/clang-tools-extra/clangd/JSONTransport.cpp +++ b/clang-tools-extra/clangd/JSONTransport.cpp @@ -7,8 +7,10 @@ //===----------------------------------------------------------------------===// #include "Logger.h" #include "Protocol.h" // For LSPError +#include "Shutdown.h" #include "Transport.h" #include "llvm/Support/Errno.h" +#include "llvm/Support/Error.h" namespace clang { namespace clangd { @@ -81,6 +83,10 @@ llvm::Error loop(MessageHandler &Handler) override { while (!feof(In)) { + if (shutdownRequested()) + return llvm::createStringError( + std::make_error_code(std::errc::operation_canceled), + "Got signal, shutting down"); if (ferror(In)) return llvm::errorCodeToError( std::error_code(errno, std::system_category())); @@ -167,7 +173,7 @@ } // Tries to read a line up to and including \n. -// If failing, feof() or ferror() will be set. +// If failing, feof(), ferror(), or shutdownRequested() will be set. bool readLine(std::FILE *In, std::string &Out) { static constexpr int BufSize = 1024; size_t Size = 0; @@ -175,7 +181,8 @@ for (;;) { Out.resize(Size + BufSize); // Handle EINTR which is sent when a debugger attaches on some platforms. - if (!llvm::sys::RetryAfterSignal(nullptr, ::fgets, &Out[Size], BufSize, In)) + if (!retryAfterSignalUnlessShutdown( + nullptr, [&] { return std::fgets(&Out[Size], BufSize, In); })) return false; clearerr(In); // If the line contained null bytes, anything after it (including \n) will @@ -190,7 +197,7 @@ } // Returns None when: -// - ferror() or feof() are set. +// - ferror(), feof(), or shutdownRequested() are set. // - Content-Length is missing or empty (protocol error) llvm::Optional JSONTransport::readStandardMessage() { // A Language Server Protocol message starts with a set of HTTP headers, @@ -244,8 +251,9 @@ std::string JSON(ContentLength, '\0'); for (size_t Pos = 0, Read; Pos < ContentLength; Pos += Read) { // Handle EINTR which is sent when a debugger attaches on some platforms. - Read = llvm::sys::RetryAfterSignal(0u, ::fread, &JSON[Pos], 1, - ContentLength - Pos, In); + Read = retryAfterSignalUnlessShutdown(0, [&]{ + return std::fread(&JSON[Pos], 1, ContentLength - Pos, In); + }); if (Read == 0) { elog("Input was aborted. Read only {0} bytes of expected {1}.", Pos, ContentLength); @@ -263,7 +271,7 @@ // - messages are delimited by '---' on a line by itself // - lines starting with # are ignored. // This is a testing path, so favor simplicity over performance here. -// When returning None, feof() or ferror() will be set. +// When returning None, feof(), ferror(), or shutdownRequested() will be set. llvm::Optional JSONTransport::readDelimitedMessage() { std::string JSON; std::string Line; @@ -280,6 +288,8 @@ JSON += Line; } + if (shutdownRequested()) + return llvm::None; if (ferror(In)) { elog("Input error while reading message!"); return llvm::None; diff --git a/clang-tools-extra/clangd/Shutdown.h b/clang-tools-extra/clangd/Shutdown.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/Shutdown.h @@ -0,0 +1,84 @@ +//===--- Shutdown.h - Unclean exit scenarios --------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// LSP specifies a protocol for shutting down: a `shutdown` request followed +// by an `exit` notification. If this protocol is followed, clangd should +// finish outstanding work and exit with code 0. +// +// The way this works in the happy case: +// - when ClangdLSPServer gets `shutdown`, it sets a flag +// - when ClangdLSPServer gets `exit`, it returns false to indicate end-of-LSP +// - Transport::loop() returns with no error +// - ClangdServer::run() checks the shutdown flag and returns with no error. +// - we `return 0` from main() +// - destructor of ClangdServer and other main()-locals runs. +// This blocks until outstanding requests complete (results are ignored) +// - global destructors run, such as fallback deletion of temporary files +// +// There are a number of things that can go wrong. Some are handled here, and +// some elsewhere. +// - `exit` notification with no `shutdown`: +// ClangdServer::run() sees this and returns false, main() returns nonzero. +// - stdin/stdout are closed +// The Transport detects this while doing IO and returns an error from loop() +// ClangdServer::run() logs a message and then returns false, etc +// - a request thread gets stuck, so the ClangdServer destructor hangs. +// Before returning from main(), we start a watchdog thread to abort() the +// process if it takes too long to exit. See abortAfterTimeout(). +// - clangd crashes (e.g. segfault or assertion) +// A fatal signal is sent (SEGV, ABRT, etc) +// The installed signal handler prints a stack trace and exits. +// - parent process goes away or tells us to shut down +// A "graceful shutdown" signal is sent (TERM, HUP, etc). +// The installed signal handler calls requestShutdown() which sets a flag. +// The Transport IO is interrupted, and Transport::loop() checks the flag and +// returns an error, etc. +// +//===----------------------------------------------------------------------===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SHUTDOWN_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SHUTDOWN_H + +#include +#include + +namespace clang { +namespace clangd { + +/// Causes this process to crash if still running after Timeout. +void abortAfterTimeout(std::chrono::seconds Timeout); + +/// Sets a flag to indicate that clangd was sent a shutdown signal, and the +/// transport loop should exit at the next opportunity. +/// If shutdown was already requested, aborts the process. +/// This function is threadsafe and signal-safe. +void requestShutdown(); +/// Checks whether requestShutdown() was called. +/// This function is threadsafe and signal-safe. +bool shutdownRequested(); + +/// Retry an operation if it gets interrupted by a signal. +/// This is like llvm::sys::RetryAfterSignal, except that if shutdown was +/// requested (which interrupts IO), we'll fail rather than retry. +template ()())> +Ret retryAfterSignalUnlessShutdown( + const typename std::enable_if::type &Fail, // Suppress deduction. + const Fun &F) { + Ret Res; + do { + if (shutdownRequested()) + return Fail; + errno = 0; + Res = F(); + } while (Res == Fail && errno == EINTR); + return Res; +} + +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/Shutdown.cpp b/clang-tools-extra/clangd/Shutdown.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/Shutdown.cpp @@ -0,0 +1,39 @@ +//===--- Shutdown.cpp - Unclean exit scenarios ----------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Shutdown.h" + +#include +#include + +namespace clang { +namespace clangd { + +void abortAfterTimeout(std::chrono::seconds Timeout) { + // This is more portable than sys::WatchDog, and yields a stack trace. + std::thread([Timeout] { + std::this_thread::sleep_for(Timeout); + std::abort(); + }).detach(); +} + +static std::atomic ShutdownRequested = {false}; + +void requestShutdown() { + if (ShutdownRequested.exchange(true)) + // This is the second shutdown request. Exit hard. + std::abort(); +} + +bool shutdownRequested() { + return ShutdownRequested; +} + +} // namespace clangd +} // namespace clang + diff --git a/clang-tools-extra/clangd/test/exit-eof.test b/clang-tools-extra/clangd/test/exit-eof.test new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/test/exit-eof.test @@ -0,0 +1,7 @@ +# RUN: not clangd -sync < %s 2> %t.err +# RUN: FileCheck %s < %t.err +# +# No LSP messages here, just let clangd see the end-of-file +# CHECK: Transport error: +# (Typically "Transport error: Input/output error" but platform-dependent). + diff --git a/clang-tools-extra/clangd/test/exit-signal.test b/clang-tools-extra/clangd/test/exit-signal.test new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/test/exit-signal.test @@ -0,0 +1,32 @@ +# This is a fiddly signal test, we need POSIX and a real shell. +UNSUPPORTED: win32 +REQUIRES: shell + +# Our goal is: +# 1) spawn clangd +# 2) wait for it to start up (install signal handlers) +# 3) send SIGTERM +# 4) wait for clangd to shut down (nonzero exit for a signal) +# 4) verify the shutdown was clean + +RUN: rm -f %t.err + # To keep clangd running, we need to hold its input stream open. + # We redirect its input to a subshell that waits for it to start up. +RUN: not clangd 2> %t.err < <( \ + # Loop waiting for clangd to start up, so signal handlers are installed. + # Reading the PID line ensures this, and lets us send a signal. +RUN: while true; do \ + # Relevant log line is I[timestamp] PID: +RUN: CLANGD_PID=$(grep -a -m 1 "PID:" %t.err | cut -d' ' -f 3); \ +RUN: [ ! -z "$CLANGD_PID" ] && break; \ +RUN: done; \ +RUN: kill $CLANGD_PID; \ + # Now wait for clangd to stop reading (before closing its input!) +RUN: while not grep "LSP finished" %t.err; do :; done; \ +RUN: ) + +# Check that clangd caught the signal and shut down cleanly. +RUN: FileCheck %s < %t.err +CHECK: Transport error: Got signal +CHECK: LSP finished + diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -11,6 +11,7 @@ #include "Features.inc" #include "Path.h" #include "Protocol.h" +#include "Shutdown.h" #include "Trace.h" #include "Transport.h" #include "index/Background.h" @@ -35,6 +36,10 @@ #include #include +#ifndef _WIN32 +#include +#endif + namespace clang { namespace clangd { namespace { @@ -435,6 +440,7 @@ llvm::InitializeAllTargetInfos(); llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); + llvm::sys::SetInterruptFunction(&requestShutdown); llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { OS << clang::getClangToolFullVersion("clangd") << "\n"; }); @@ -531,6 +537,10 @@ LoggingSession LoggingSession(Logger); // Write some initial logs before we start doing any real work. log("{0}", clang::getClangToolFullVersion("clangd")); +// FIXME: abstract this better, and print PID on windows too. +#ifndef _WIN32 + log("PID: {0}", getpid()); +#endif { SmallString<128> CWD; if (auto Err = llvm::sys::fs::current_path(CWD)) @@ -683,12 +693,7 @@ // However if a bug causes them to run forever, we want to ensure the process // eventually exits. As clangd isn't directly user-facing, an editor can // "leak" clangd processes. Crashing in this case contains the damage. - // - // This is more portable than sys::WatchDog, and yields a stack trace. - std::thread([] { - std::this_thread::sleep_for(std::chrono::minutes(5)); - std::abort(); - }).detach(); + abortAfterTimeout(std::chrono::minutes(5)); return ExitCode; }