Commit 19ac0eaf authored by Sam McCall's avatar Sam McCall
Browse files

[clangd] Shutdown cleanly on signals.

Summary:
This avoids leaking PCH files if editors don't use the LSP shutdown protocol.

This is one fix for https://github.com/clangd/clangd/issues/209
(Though I think we should *also* be unlinking the files)

Reviewers: kadircet, jfb

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70684
parent 31c25fad
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -69,6 +69,7 @@ add_clang_library(clangDaemon
  Selection.cpp
  SemanticHighlighting.cpp
  SemanticSelection.cpp
  Shutdown.cpp
  SourceCode.cpp
  QueryDriverDatabase.cpp
  Threading.cpp
+16 −6
Original line number Diff line number Diff line
@@ -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 @@ public:

  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 @@ bool JSONTransport::handleMessage(llvm::json::Value Message,
}

// 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 @@ bool readLine(std::FILE *In, std::string &Out) {
  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 @@ bool readLine(std::FILE *In, std::string &Out) {
}

// Returns None when:
//  - ferror() or feof() are set.
//  - ferror(), feof(), or shutdownRequested() are set.
//  - Content-Length is missing or empty (protocol error)
llvm::Optional<std::string> JSONTransport::readStandardMessage() {
  // A Language Server Protocol message starts with a set of HTTP headers,
@@ -244,8 +251,9 @@ llvm::Optional<std::string> JSONTransport::readStandardMessage() {
  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 @@ llvm::Optional<std::string> JSONTransport::readStandardMessage() {
// - 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<std::string> JSONTransport::readDelimitedMessage() {
  std::string JSON;
  std::string Line;
@@ -280,6 +288,8 @@ llvm::Optional<std::string> JSONTransport::readDelimitedMessage() {
    JSON += Line;
  }

  if (shutdownRequested())
    return llvm::None;
  if (ferror(In)) {
    elog("Input error while reading message!");
    return llvm::None;
+39 −0
Original line number Diff line number Diff line
//===--- 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 <atomic>
#include <thread>

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<bool> 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
+84 −0
Original line number Diff line number Diff line
//===--- 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 <cerrno>
#include <chrono>

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 <typename Fun, typename Ret = decltype(std::declval<Fun>()())>
Ret retryAfterSignalUnlessShutdown(
    const typename std::enable_if<true, Ret>::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
+7 −0
Original line number Diff line number Diff line
# 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).
Loading