This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bda5f20674d: [clangd] abort if shutdown takes more than a
minute. (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D69329?vs=226135&id=226148#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69329/new/
https://reviews.llvm.org/D69329
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -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 @@
/*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;
}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -44,6 +44,7 @@
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 @@
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
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1231,7 +1231,11 @@
// 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 @@
CleanExit = false;
}
- // Destroy ClangdServer to ensure all worker threads finish.
- Server.reset();
return CleanExit && ShutdownRequestReceived;
}
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -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 @@
/*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;
}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -44,6 +44,7 @@
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 @@
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
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1231,7 +1231,11 @@
// 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 @@
CleanExit = false;
}
- // Destroy ClangdServer to ensure all worker threads finish.
- Server.reset();
return CleanExit && ShutdownRequestReceived;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
