[clangd] abort if shutdown takes more than a minute.

Summary:
A certain class of bug (e.g. infloop on an AST worker thread) currently means
clangd never terminates, even if the editor shuts down the protocol and closes
our stdin, and the main thread recognizes that.

Instead, let's wait 60 seconds for threads to finish cleanly, and then crash
if they haven't.

(Obviously, we should still fix these bugs).

Reviewers: kadircet

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

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69329
This commit is contained in:
Sam McCall 2019-10-23 11:11:18 +02:00
parent 60286818ff
commit 8bda5f2067
3 changed files with 29 additions and 8 deletions

View File

@ -1231,7 +1231,11 @@ ClangdLSPServer::ClangdLSPServer(
// clang-format on
}
ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true; }
ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true;
// Explicitly destroy ClangdServer first, blocking on threads it owns.
// This ensures they don't access any other members.
Server.reset();
}
bool ClangdLSPServer::run() {
// Run the Language Server loop.
@ -1241,8 +1245,6 @@ bool ClangdLSPServer::run() {
CleanExit = false;
}
// Destroy ClangdServer to ensure all worker threads finish.
Server.reset();
return CleanExit && ShutdownRequestReceived;
}

View File

@ -44,6 +44,7 @@ public:
llvm::Optional<Path> CompileCommandsDir, bool UseDirBasedCDB,
llvm::Optional<OffsetEncoding> ForcedOffsetEncoding,
const ClangdServer::Options &Opts);
/// The destructor blocks on any outstanding background tasks.
~ClangdLSPServer();
/// Run LSP server loop, communicating with the Transport provided in the
@ -211,11 +212,10 @@ private:
std::unique_ptr<GlobalCompilationDatabase> BaseCDB;
// CDB is BaseCDB plus any comands overridden via LSP extensions.
llvm::Optional<OverlayCDB> CDB;
// The ClangdServer is created by the "initialize" LSP method.
// It is destroyed before run() returns, to ensure worker threads exit.
ClangdServer::Options ClangdServerOpts;
llvm::Optional<ClangdServer> Server;
llvm::Optional<OffsetEncoding> NegotiatedOffsetEncoding;
// The ClangdServer is created by the "initialize" LSP method.
llvm::Optional<ClangdServer> Server;
};
} // namespace clangd
} // namespace clang

View File

@ -27,6 +27,7 @@
#include "llvm/Support/Signals.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/Support/raw_ostream.h"
#include <chrono>
#include <cstdlib>
#include <iostream>
#include <memory>
@ -670,6 +671,24 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
/*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
OffsetEncodingFromFlag, Opts);
llvm::set_thread_name("clangd.main");
return LSPServer.run() ? 0
: static_cast<int>(ErrorResultCode::NoShutdownRequest);
int ExitCode = LSPServer.run()
? 0
: static_cast<int>(ErrorResultCode::NoShutdownRequest);
log("LSP finished, exiting with status {0}", ExitCode);
// There may still be lingering background threads (e.g. slow requests
// whose results will be dropped, background index shutting down).
//
// These should terminate quickly, and ~ClangdLSPServer blocks on them.
// 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();
return ExitCode;
}