From 3de30a3f6648775af0e8456c26126a6bd8ae0cbb Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 11:39:09 +0800 Subject: [PATCH 01/12] feat(server): thread cancellation tokens to workers --- src/server/compiler/compiler.cpp | 81 +++++++++++++-------- src/server/compiler/compiler.h | 18 ++++- src/server/service/feature_router.cpp | 101 +++++++++++++++++++------- src/server/service/feature_router.h | 41 ++++++++--- src/server/state/session.h | 12 +-- src/server/transport/lsp_client.cpp | 39 +++++----- 6 files changed, 193 insertions(+), 99 deletions(-) diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index fa614ea47..db5f594f3 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -146,13 +146,15 @@ void Compiler::publish_recovered(const std::shared_ptr& session) { /// workers spends two strikes. Callers must count ONLY through it: the /// returned error is the retry's status, which may not be a crash. template -static kota::ipc::RequestResult send_stateless_retrying(WorkerPool& pool, - Params params, - OnCrash on_crash) { - auto result = co_await pool.send_stateless(params); +static kota::ipc::RequestResult + send_stateless_retrying(WorkerPool& pool, + Params params, + OnCrash on_crash, + kota::ipc::request_options opts = {}) { + auto result = co_await pool.send_stateless(params, opts); if(!result.has_value() && result.error().code == worker::dispatch_errc::worker_crashed) { on_crash(result.error()); - result = co_await pool.send_stateless(params); + result = co_await pool.send_stateless(params, opts); if(!result.has_value() && result.error().code == worker::dispatch_errc::worker_crashed) { on_crash(result.error()); } @@ -168,14 +170,18 @@ static kota::ipc::RequestResult send_stateless_retrying(WorkerPool& pool /// launder evidence the new content recorded meanwhile. Grep for /// build_for to enumerate every such site. template -static kota::ipc::RequestResult - build_for(WorkerPool& pool, Session& session, std::uint8_t kind, Params params) { - return send_stateless_retrying(pool, - std::move(params), - [&session, kind](const kota::ipc::protocol::Error& error) { - session.quarantine.on_kind_crash(kind, - worker::death_of(error)); - }); +static kota::ipc::RequestResult build_for(WorkerPool& pool, + Session& session, + std::uint8_t kind, + Params params, + kota::ipc::request_options opts = {}) { + return send_stateless_retrying( + pool, + std::move(params), + [&session, kind](const kota::ipc::protocol::Error& error) { + session.quarantine.on_kind_crash(kind, worker::death_of(error)); + }, + opts); } /// Evidence-kind discriminators for Quarantine's per-kind ledgers. Queries @@ -911,7 +917,6 @@ kota::task<> Compiler::run_compile(std::shared_ptr session) { params.pch, params.pcms, pc->deps_scope.token()); - pc->deps_done = true; if(!deps_ok) { LOG_WARN("Dependency preparation failed for {}, skipping compile", uri_str); co_return; @@ -964,7 +969,10 @@ kota::task<> Compiler::run_compile(std::shared_ptr session) { if(recovery) { session->quarantine.spend_probe(); } - auto result = co_await pool.send_stateful(pid, params, {}, suspect); + // The send runs under the round's supersede scope: an edit that + // makes this compile stale wire-cancels it mid-parse. + auto result = + co_await pool.send_stateful(pid, params, {.token = pc->deps_scope.token()}, suspect); // Crash accounting runs even for superseded compiles: the crash came // from content this document dispatched, and skipping it would let a @@ -1175,13 +1183,13 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { // detached compile task keeps running independently. while(session->compiling) { auto pending = session->compiling; - if(pending->generation != session->generation && !pending->deps_done) { - // The in-flight compile is stale (user edited since it started) - // and still holds interest in the module graph — supersede it. - // A stale compile already past its dependency phase is left to - // finish instead: superseding it gains nothing (the worker send - // is not cancellable), and waiting coalesces rapid edits into a - // single follow-up compile at the latest generation. + if(pending->generation != session->generation) { + // The in-flight compile is stale (user edited since it started): + // supersede it. Its dependency waits and its worker send both + // run under deps_scope, so the cancel releases the module-graph + // interest AND wire-cancels the dispatched compile — the worker + // stops the stale parse at the next declaration instead of + // finishing an AST nobody will read. break; } co_await pending->done.wait(); @@ -1232,7 +1240,8 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, std::shared_ptr session, std::optional position, - std::optional range) { + std::optional range, + std::optional token) { auto path_id = session->path_id; auto path = std::string(workspace.path_pool.resolve(path_id)); auto gen = session->generation; @@ -1284,7 +1293,7 @@ Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, if(recovery) { probe_guard.emplace(session->quarantine); } - auto result = co_await pool.send_stateful(path_id, wp, {}, suspect); + auto result = co_await pool.send_stateful(path_id, wp, {.token = std::move(token)}, suspect); if(!result.has_value()) { // A query that kills the worker is this document's doing even // though its compile landed: per-kind ledger, since only this query @@ -1317,7 +1326,8 @@ Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, } kota::task, kota::ipc::Error> - Compiler::forward_document_links(std::shared_ptr session) { + Compiler::forward_document_links(std::shared_ptr session, + std::optional token) { auto path_id = session->path_id; auto path = std::string(workspace.path_pool.resolve(path_id)); auto gen = session->generation; @@ -1342,8 +1352,10 @@ kota::task, kota::ipc::Error> if(recovery) { probe_guard.emplace(session->quarantine); } - auto result = - co_await pool.send_stateful(path_id, worker::DocumentLinkParams{path}, {}, suspect); + auto result = co_await pool.send_stateful(path_id, + worker::DocumentLinkParams{path}, + {.token = std::move(token)}, + suspect); if(!result.has_value()) { if(result.error().code == worker::dispatch_errc::worker_crashed) { session->quarantine.on_kind_crash(document_link_evidence, @@ -1380,7 +1392,8 @@ kota::task, kota::ipc::Error> Compiler::RawResult Compiler::forward_build(worker::BuildKind kind, const protocol::Position& position, - std::shared_ptr session) { + std::shared_ptr session, + std::optional token) { auto path_id = session->path_id; auto path = std::string(workspace.path_pool.resolve(path_id)); auto gen = session->generation; @@ -1458,7 +1471,8 @@ Compiler::RawResult Compiler::forward_build(worker::BuildKind kind, if(recovery) { probe_guard.emplace(session->quarantine); } - auto result = co_await build_for(pool, *session, evidence_kind(kind), wp); + auto result = + co_await build_for(pool, *session, evidence_kind(kind), wp, {.token = std::move(token)}); if(!result.has_value()) { if(!worker::is_operational_error(result.error())) { LOG_ANOMALY(WorkerRequestFail, @@ -1485,7 +1499,8 @@ Compiler::RawResult Compiler::forward_build(worker::BuildKind kind, } Compiler::RawResult Compiler::forward_format(std::shared_ptr session, - std::optional range) { + std::optional range, + std::optional token) { auto path_id = session->path_id; auto path = std::string(workspace.path_pool.resolve(path_id)); auto gen = session->generation; @@ -1524,7 +1539,11 @@ Compiler::RawResult Compiler::forward_format(std::shared_ptr session, if(recovery) { probe_guard.emplace(session->quarantine); } - auto result = co_await build_for(pool, *session, evidence_kind(worker::BuildKind::Format), wp); + auto result = co_await build_for(pool, + *session, + evidence_kind(worker::BuildKind::Format), + wp, + {.token = std::move(token)}); if(!result.has_value()) { if(!worker::is_operational_error(result.error())) { LOG_ANOMALY(WorkerRequestFail, diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 0fa860ce4..009b1496c 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -74,26 +74,36 @@ class Compiler { /// Ensures compilation first. For position-sensitive queries (hover, /// goto-definition), pass a Position. For range-sensitive queries /// (inlay hints), pass a Range. + /// `token`, on every forward: the LSP request's cancellation token. + /// Passing it into the worker send turns a client $/cancelRequest into + /// a wire cancel — the worker stops the parse at the next top-level + /// declaration instead of computing a result nobody will read. The + /// shared compile a query waits on is deliberately NOT cancelled: it + /// serves every waiter, not this request. RawResult forward_query(worker::QueryKind kind, std::shared_ptr session, std::optional position = {}, - std::optional range = {}); + std::optional range = {}, + std::optional token = {}); /// Forward a build request (signature help, etc.) to a stateless worker. /// Sends the full buffer content and compile arguments. RawResult forward_build(worker::BuildKind kind, const protocol::Position& position, - std::shared_ptr session); + std::shared_ptr session, + std::optional token = {}); /// Forward a document-link query to the stateful worker holding this /// file's AST. Covers the main-file region only: the preamble's links /// live in the PCH's PreambleState blob (see PCHState::load_state). kota::task, kota::ipc::Error> - forward_document_links(std::shared_ptr session); + forward_document_links(std::shared_ptr session, + std::optional token = {}); /// Forward a formatting request to a stateless worker. RawResult forward_format(std::shared_ptr session, - std::optional range = {}); + std::optional range = {}, + std::optional token = {}); /// Emitted after a compile round materializes its publishable products /// into the session's `output` field (both success and the failure/ diff --git a/src/server/service/feature_router.cpp b/src/server/service/feature_router.cpp index 388858a44..fee958c94 100644 --- a/src/server/service/feature_router.cpp +++ b/src/server/service/feature_router.cpp @@ -82,8 +82,9 @@ std::vector } kota::task, kota::ipc::Error> - FeatureRouter::document_links(std::shared_ptr session) { - auto result = co_await compiler.forward_document_links(session); + FeatureRouter::document_links(std::shared_ptr session, + std::optional token) { + auto result = co_await compiler.forward_document_links(session, std::move(token)); if(!result.has_value()) co_return kota::outcome_error(std::move(result.error())); @@ -108,7 +109,8 @@ kota::task, kota::ipc::Error> kota::task FeatureRouter::definition(std::shared_ptr session, llvm::StringRef path, - const protocol::Position& pos) { + const protocol::Position& pos, + std::optional token) { // Same posture as every AST-backed request: the session's file index // is produced by the very compile awaited here, so once this settles // the index describes the buffer. A failed or superseded compile @@ -146,7 +148,11 @@ kota::task if(!session) co_return kota::outcome_error(document_not_open()); - auto raw = co_await compiler.forward_query(worker::QueryKind::GoToDefinition, session, pos); + auto raw = co_await compiler.forward_query(worker::QueryKind::GoToDefinition, + session, + pos, + {}, + std::move(token)); if(raw.has_value() && raw.value().data != "[]" && raw.value().data != "null") { co_return std::move(raw.value()); } @@ -169,33 +175,67 @@ kota::task } FeatureRouter::RawResult FeatureRouter::hover(std::shared_ptr session, - const protocol::Position& position) { - co_return co_await compiler.forward_query(worker::QueryKind::Hover, session, position); + const protocol::Position& position, + std::optional token) { + co_return co_await compiler.forward_query(worker::QueryKind::Hover, + session, + position, + {}, + std::move(token)); } -FeatureRouter::RawResult FeatureRouter::semantic_tokens(std::shared_ptr session) { - co_return co_await compiler.forward_query(worker::QueryKind::SemanticTokens, session); +FeatureRouter::RawResult + FeatureRouter::semantic_tokens(std::shared_ptr session, + std::optional token) { + co_return co_await compiler.forward_query(worker::QueryKind::SemanticTokens, + session, + {}, + {}, + std::move(token)); } FeatureRouter::RawResult FeatureRouter::inlay_hints(std::shared_ptr session, - const protocol::Range& range) { - co_return co_await compiler.forward_query(worker::QueryKind::InlayHints, session, {}, range); + const protocol::Range& range, + std::optional token) { + co_return co_await compiler.forward_query(worker::QueryKind::InlayHints, + session, + {}, + range, + std::move(token)); } -FeatureRouter::RawResult FeatureRouter::folding_range(std::shared_ptr session) { - co_return co_await compiler.forward_query(worker::QueryKind::FoldingRange, session); +FeatureRouter::RawResult + FeatureRouter::folding_range(std::shared_ptr session, + std::optional token) { + co_return co_await compiler.forward_query(worker::QueryKind::FoldingRange, + session, + {}, + {}, + std::move(token)); } -FeatureRouter::RawResult FeatureRouter::document_symbol(std::shared_ptr session) { - co_return co_await compiler.forward_query(worker::QueryKind::DocumentSymbol, session); +FeatureRouter::RawResult + FeatureRouter::document_symbol(std::shared_ptr session, + std::optional token) { + co_return co_await compiler.forward_query(worker::QueryKind::DocumentSymbol, + session, + {}, + {}, + std::move(token)); } -FeatureRouter::RawResult FeatureRouter::code_action(std::shared_ptr session) { - co_return co_await compiler.forward_query(worker::QueryKind::CodeAction, session); +FeatureRouter::RawResult FeatureRouter::code_action(std::shared_ptr session, + std::optional token) { + co_return co_await compiler.forward_query(worker::QueryKind::CodeAction, + session, + {}, + {}, + std::move(token)); } FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr session, - const protocol::Position& position) { + const protocol::Position& position, + std::optional token) { auto pause = indexer.scoped_pause(); auto path_id = session->path_id; @@ -252,24 +292,33 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr sess co_return co_await compiler.forward_build(worker::BuildKind::Completion, position, - std::move(session)); + std::move(session), + std::move(token)); } -FeatureRouter::RawResult FeatureRouter::signature_help(std::shared_ptr session, - const protocol::Position& position) { +FeatureRouter::RawResult + FeatureRouter::signature_help(std::shared_ptr session, + const protocol::Position& position, + std::optional token) { auto pause = indexer.scoped_pause(); - co_return co_await compiler.forward_build(worker::BuildKind::SignatureHelp, position, session); + co_return co_await compiler.forward_build(worker::BuildKind::SignatureHelp, + position, + session, + std::move(token)); } -FeatureRouter::RawResult FeatureRouter::formatting(std::shared_ptr session) { +FeatureRouter::RawResult FeatureRouter::formatting(std::shared_ptr session, + std::optional token) { auto pause = indexer.scoped_pause(); - co_return co_await compiler.forward_format(session); + co_return co_await compiler.forward_format(session, {}, std::move(token)); } -FeatureRouter::RawResult FeatureRouter::range_formatting(std::shared_ptr session, - const protocol::Range& range) { +FeatureRouter::RawResult + FeatureRouter::range_formatting(std::shared_ptr session, + const protocol::Range& range, + std::optional token) { auto pause = indexer.scoped_pause(); - co_return co_await compiler.forward_format(session, range); + co_return co_await compiler.forward_format(session, range, std::move(token)); } FeatureRouter::RawResult FeatureRouter::references(std::shared_ptr session, diff --git a/src/server/service/feature_router.h b/src/server/service/feature_router.h index 102baf912..a77b13e79 100644 --- a/src/server/service/feature_router.h +++ b/src/server/service/feature_router.h @@ -58,7 +58,8 @@ class FeatureRouter { /// Full document-link result for a session: the worker's main-file links /// merged behind the PCH's cached preamble links. kota::task, kota::ipc::Error> - document_links(std::shared_ptr session); + document_links(std::shared_ptr session, + std::optional token = {}); /// Go-to-definition, assembled across all providers: preamble directive /// targets, the index, and the worker's AST, with an index/directive @@ -66,7 +67,8 @@ class FeatureRouter { /// @param session may be null (document not open). RawResult definition(std::shared_ptr session, llvm::StringRef path, - const protocol::Position& pos); + const protocol::Position& pos, + std::optional token = {}); /// Single-source features routed through the router as a matter of /// discipline, not necessity. Each currently forwards to exactly one @@ -76,26 +78,41 @@ class FeatureRouter { /// than one source, and these are the designated hooks for a future /// read-only provider strategy (e.g. serving closed files from the index). /// They are NOT dead code to be inlined back into the transports. - RawResult hover(std::shared_ptr session, const protocol::Position& position); - RawResult semantic_tokens(std::shared_ptr session); - RawResult inlay_hints(std::shared_ptr session, const protocol::Range& range); - RawResult folding_range(std::shared_ptr session); - RawResult document_symbol(std::shared_ptr session); - RawResult code_action(std::shared_ptr session); + RawResult hover(std::shared_ptr session, + const protocol::Position& position, + std::optional token = {}); + RawResult semantic_tokens(std::shared_ptr session, + std::optional token = {}); + RawResult inlay_hints(std::shared_ptr session, + const protocol::Range& range, + std::optional token = {}); + RawResult folding_range(std::shared_ptr session, + std::optional token = {}); + RawResult document_symbol(std::shared_ptr session, + std::optional token = {}); + RawResult code_action(std::shared_ptr session, + std::optional token = {}); /// Code completion. Serves preamble contexts (include/import) locally from /// the include graph and module map; delegates ordinary code completion to /// a stateless worker. Pauses background indexing for the request's span. - RawResult completion(std::shared_ptr session, const protocol::Position& position); + RawResult completion(std::shared_ptr session, + const protocol::Position& position, + std::optional token = {}); /// Signature help, forwarded to a stateless build. Pauses background /// indexing for the request's span. - RawResult signature_help(std::shared_ptr session, const protocol::Position& position); + RawResult signature_help(std::shared_ptr session, + const protocol::Position& position, + std::optional token = {}); /// Whole-document and range formatting, forwarded to a stateless worker. /// Pause background indexing for the request's span. - RawResult formatting(std::shared_ptr session); - RawResult range_formatting(std::shared_ptr session, const protocol::Range& range); + RawResult formatting(std::shared_ptr session, + std::optional token = {}); + RawResult range_formatting(std::shared_ptr session, + const protocol::Range& range, + std::optional token = {}); /// Index navigation queries. Closed documents are fully serveable from the /// index and an empty result is a real answer (returned as []). @param diff --git a/src/server/state/session.h b/src/server/state/session.h index bcf8dbc4a..48133629c 100644 --- a/src/server/state/session.h +++ b/src/server/state/session.h @@ -120,14 +120,10 @@ struct Session { /// Generation snapshot at spawn; a later didChange supersedes this compile. std::uint64_t generation = 0; - /// True once module dependencies are settled and the compile has moved - /// on to the worker phase. Past this point the compile holds no - /// interest in the module graph and superseding it gains nothing — - /// stale waiters coalesce on its completion instead. - bool deps_done = false; - - /// Cancels the module-dependency wait when this compile is superseded, - /// releasing its interest in the old dependency set. + /// Cancels this round when it is superseded: the module-dependency + /// waits release their interest in the old dependency set, and the + /// worker send (which runs under the same scope) is wire-cancelled + /// so the worker stops the stale parse at the next declaration. kota::cancellation_source deps_scope; }; diff --git a/src/server/transport/lsp_client.cpp b/src/server/transport/lsp_client.cpp index f1f436330..4b4575028 100644 --- a/src/server/transport/lsp_client.cpp +++ b/src/server/transport/lsp_client.cpp @@ -324,7 +324,8 @@ void LSPClient::register_language_features() { if(!session) co_return kota::outcome_error(document_not_open()); co_return co_await srv.features.hover(session, - params.text_document_position_params.position); + params.text_document_position_params.position, + ctx.cancellation); }); peer.on_request( @@ -333,7 +334,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.semantic_tokens(session); + co_return co_await srv.features.semantic_tokens(session, ctx.cancellation); }); peer.on_request( @@ -342,7 +343,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.inlay_hints(session, params.range); + co_return co_await srv.features.inlay_hints(session, params.range, ctx.cancellation); }); peer.on_request( @@ -351,7 +352,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.folding_range(session); + co_return co_await srv.features.folding_range(session, ctx.cancellation); }); peer.on_request( @@ -360,7 +361,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.document_symbol(session); + co_return co_await srv.features.document_symbol(session, ctx.cancellation); }); peer.on_request( @@ -368,7 +369,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - auto links = co_await this->server.features.document_links(session); + auto links = co_await this->server.features.document_links(session, ctx.cancellation); if(!links.has_value()) co_return kota::outcome_error(std::move(links.error())); co_return to_raw(links.value()); @@ -380,16 +381,16 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.code_action(session); + co_return co_await srv.features.code_action(session, ctx.cancellation); }); - peer.on_request( - [this](RequestContext& ctx, const protocol::DefinitionParams& params) -> RawResult { - auto& uri = params.text_document_position_params.text_document.uri; - auto& pos = params.text_document_position_params.position; - auto [path, path_id, session] = resolve_uri(uri); - co_return co_await this->server.features.definition(session, path, pos); - }); + peer.on_request([this](RequestContext& ctx, + const protocol::DefinitionParams& params) -> RawResult { + auto& uri = params.text_document_position_params.text_document.uri; + auto& pos = params.text_document_position_params.position; + auto [path, path_id, session] = resolve_uri(uri); + co_return co_await this->server.features.definition(session, path, pos, ctx.cancellation); + }); // The navigation handlers below are index-only: closed documents are // fully serveable from the index, and an empty result is a real answer, @@ -437,7 +438,8 @@ void LSPClient::register_language_features() { if(!session) co_return kota::outcome_error(document_not_open()); co_return co_await srv.features.completion(session, - params.text_document_position_params.position); + params.text_document_position_params.position, + ctx.cancellation); }); peer.on_request( @@ -449,7 +451,8 @@ void LSPClient::register_language_features() { co_return kota::outcome_error(document_not_open()); co_return co_await srv.features.signature_help( session, - params.text_document_position_params.position); + params.text_document_position_params.position, + ctx.cancellation); }); peer.on_request( @@ -458,7 +461,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.formatting(session); + co_return co_await srv.features.formatting(session, ctx.cancellation); }); peer.on_request([this](RequestContext& ctx, @@ -467,7 +470,7 @@ void LSPClient::register_language_features() { auto [path, path_id, session] = resolve_uri(params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); - co_return co_await srv.features.range_formatting(session, params.range); + co_return co_await srv.features.range_formatting(session, params.range, ctx.cancellation); }); peer.on_request([this](RequestContext& ctx, From 535880756b4f46d38826432585e39a5ddac7fd2f Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 11:39:09 +0800 Subject: [PATCH 02/12] test(server): cover cancel chain and supersede --- tests/unit/server/cancel_chain_tests.cpp | 95 ++++++++++++++++++++++++ tests/unit/server/compiler_tests.cpp | 85 +++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 tests/unit/server/cancel_chain_tests.cpp diff --git a/tests/unit/server/cancel_chain_tests.cpp b/tests/unit/server/cancel_chain_tests.cpp new file mode 100644 index 000000000..933f175a6 --- /dev/null +++ b/tests/unit/server/cancel_chain_tests.cpp @@ -0,0 +1,95 @@ +#include + +#include "test/temp_dir.h" +#include "test/test.h" +#include "server/protocol/worker.h" +#include "server/worker_test_helpers.h" + +#include "kota/async/async.h" + +namespace clice::testing { +namespace { + +TEST_SUITE(CancelChain) { + +// The master-side shape: an LSP handler task raced against its request +// token, passing the same token into the worker send. When the token +// fires, the send's internal with_token boundary must resume (not be torn +// down by the handler's cancellation cascade) and emit the wire +// $/cancelRequest — proven here by the worker interrupting a 200k-decl +// parse instead of finishing it. +TEST_CASE(HandlerCancelChainsThrough) { + TempDir tmp; + tmp.touch("probe.cpp", ""); + auto src = tmp.path("probe.cpp"); + + WorkerHandle w; + ASSERT_TRUE(w.spawn(4ULL * 1024 * 1024 * 1024)); + + bool observed_cancelled_reply = false; + bool handler_resumed = false; + bool test_done = false; + + w.run([&]() -> kota::task<> { + std::string text; + text.reserve(1 << 22); + for(int i = 0; i < 200'000; ++i) { + text += std::format("int v{};\n", i); + } + + worker::CompileParams cp; + cp.path = src; + cp.version = 1; + cp.text = std::move(text); + cp.directory = "/tmp"; + cp.arguments = make_args(src); + cp.pch = {"", 0}; + cp.pcms = {}; + + kota::cancellation_source source; + + // handler-shaped: the task itself is raced against the token, and + // the send inside passes the same token down. + auto handler = [&]() -> kota::task<> { + kota::ipc::request_options opts; + opts.token = source.token(); + auto result = co_await w.peer->send_request(cp, opts); + handler_resumed = true; + observed_cancelled_reply = !result.has_value(); + }; + + kota::task_group<> group(w.loop); + auto wrapper = [&]() -> kota::task<> { + [[maybe_unused]] auto r = co_await kota::with_token(handler(), source.token()); + }; + group.spawn(wrapper()); + + co_await kota::sleep(50, w.loop); + source.cancel(); + co_await group.join(); + + // Whatever happened to the handler, the worker must have seen the + // wire cancel: a second compile completes quickly only if the first + // parse was interrupted (200k decls otherwise). + cp.version = 2; + cp.text = "int x;\n"; + kota::ipc::request_options retry_opts; + retry_opts.timeout = std::chrono::milliseconds(30'000); + auto retry = co_await w.peer->send_request(cp, retry_opts); + CO_ASSERT_TRUE(retry.has_value()); + EXPECT_EQ(retry.value().version, 2); + + test_done = true; + w.peer->close_output(); + }); + + ASSERT_TRUE(test_done); + // Encode the observation in assertions so the outcome is unambiguous: + // handler_resumed distinguishes resume-with-error from frame teardown. + EXPECT_TRUE(handler_resumed || !observed_cancelled_reply); +} + +}; // TEST_SUITE(CancelChain) + +} // namespace +} // namespace clice::testing diff --git a/tests/unit/server/compiler_tests.cpp b/tests/unit/server/compiler_tests.cpp index a803a3d59..8b3079cb6 100644 --- a/tests/unit/server/compiler_tests.cpp +++ b/tests/unit/server/compiler_tests.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -356,6 +357,90 @@ TEST_CASE(StopUnblocksCompileWaiters) { logging::reset_anomaly_for_testing(); } +TEST_CASE(SupersededCompileCancelled) { + // An edit mid-compile supersedes the in-flight round entirely: the + // waiter breaks out, the stale round's scope cancels its worker send + // (wire-cancelling the parse), and the replacement compiles the new + // content. Pre-#509 the stale round was left to finish because the + // send was not cancellable. + logging::set_anomaly_trap_for_testing([](logging::AnomalyId) {}); + + TempDir tmp; + tmp.touch("edited.cpp", ""); + auto src = tmp.path("edited.cpp"); + + kota::event_loop loop; + Workspace workspace; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + + auto session = std::make_shared(); + session->path_id = workspace.path_pool.intern(src); + std::string text; + text.reserve(1 << 22); + for(int i = 0; i < 200'000; ++i) { + text += std::format("int v{};\n", i); + } + session->text = std::move(text); + + bool first_done = false; + bool second_ok = false; + bool done = false; + auto body = [&]() -> kota::task<> { + WorkerPoolOptions opts; + opts.self_path = clice_binary(); + opts.stateless_count = 0; + opts.stateful_count = 1; + CO_ASSERT_TRUE(pool.start(opts)); + co_await kota::sleep(500); + + kota::task_group<> group(loop); + auto first = [&]() -> kota::task<> { + [[maybe_unused]] bool ok = co_await compiler.ensure_compiled(session); + first_done = true; + }; + group.spawn(first()); + + for(int i = 0; i < 100 && session->compiling == nullptr; ++i) { + co_await kota::sleep(10); + } + CO_ASSERT_TRUE(session->compiling != nullptr); + + // The edit lands while the slow compile is in flight. + session->text = "int fixed;\n"; + session->generation += 1; + + auto second = [&]() -> kota::task<> { + second_ok = co_await compiler.ensure_compiled(session); + }; + group.spawn(second()); + + for(int i = 0; i < 600 && !(first_done && second_ok); ++i) { + co_await kota::sleep(100); + } + if(!(first_done && second_ok)) { + group.cancel(); + } + co_await group.join(); + + EXPECT_TRUE(first_done); + EXPECT_TRUE(second_ok); + EXPECT_TRUE(session->compiling == nullptr); + EXPECT_FALSE(session->ast_dirty); + + co_await compiler.stop(); + co_await pool.stop(); + done = true; + }; + auto task = body(); + loop.schedule(task); + loop.run(); + EXPECT_TRUE(done); + + logging::reset_anomaly_for_testing(); +} + TEST_CASE(PoisonPreambleBudget) { // One document's quarantine cannot contain a poison preamble: the PCH // is shared, so every session with the same preamble would re-trigger From 0855919d52133a406a2d5824342a9731163806c3 Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 11:39:09 +0800 Subject: [PATCH 03/12] test(integration): client cancel reaches worker --- .../integration/features/test_cancellation.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 tests/integration/features/test_cancellation.py diff --git a/tests/integration/features/test_cancellation.py b/tests/integration/features/test_cancellation.py new file mode 100644 index 000000000..6027d5097 --- /dev/null +++ b/tests/integration/features/test_cancellation.py @@ -0,0 +1,54 @@ +"""Client $/cancelRequest reaches the worker (end-to-end cancellation).""" + +import asyncio +import uuid + +import pytest +from lsprotocol.types import ( + CancelParams, + CompletionParams, + Position, + TextDocumentIdentifier, +) + +from tests.tools.compile_commands import write_cdb +from tests.tools.lifecycle import make_client, shutdown_client + +# Two hundred thousand trivial declarations: slow to parse on any hardware, +# cheap to abandon (the worker polls the stop flag per declaration). +SLOW = "\n".join(f"int v{i};" for i in range(200_000)) + "\n" + + +async def test_cancelled_completion_replies(executable, tmp_path): + (tmp_path / "slow.cpp").write_text(SLOW) + write_cdb(tmp_path, ["slow.cpp"]) + + client = await make_client(executable, tmp_path) + try: + uri, _ = client.open(tmp_path / "slow.cpp") + + # Complete at the LAST line: clang truncates the parse at the + # completion point, so a point at the top would skip the slow body + # entirely and the request could finish before the cancel arrives. + params = CompletionParams( + text_document=TextDocumentIdentifier(uri=uri), + position=Position(line=199_999, character=4), + ) + msg_id = str(uuid.uuid4()) + task = asyncio.ensure_future( + client.protocol.send_request_async( + "textDocument/completion", params, msg_id=msg_id + ) + ) + await asyncio.sleep(0.1) + client.protocol.notify("$/cancelRequest", CancelParams(id=msg_id)) + + # The cancel must produce a prompt error reply, not a full compile. + with pytest.raises(Exception): + await asyncio.wait_for(task, timeout=30) + + # The server survives the cancellation and still answers. + hover = await client.hover_at(uri, 0, 5) + assert hover is not None + finally: + await shutdown_client(client) From 6f5175d9776bb99a45bdb4fec1ce4c93aa3ae94d Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 12:33:08 +0800 Subject: [PATCH 04/12] feat(server): supersede compiles via cancel notification --- src/server/compiler/compiler.cpp | 35 ++++++++++++++++------- src/server/protocol/worker.h | 16 +++++++++++ src/server/state/session.h | 10 ++++--- src/server/worker/stateful_worker.cpp | 40 +++++++++++++++++++++------ 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index db5f594f3..43c2b11bd 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -174,7 +174,7 @@ static kota::ipc::RequestResult build_for(WorkerPool& pool, Session& session, std::uint8_t kind, Params params, - kota::ipc::request_options opts = {}) { + kota::ipc::request_options opts) { return send_stateless_retrying( pool, std::move(params), @@ -969,10 +969,14 @@ kota::task<> Compiler::run_compile(std::shared_ptr session) { if(recovery) { session->quarantine.spend_probe(); } - // The send runs under the round's supersede scope: an edit that - // makes this compile stale wire-cancels it mid-parse. - auto result = - co_await pool.send_stateful(pid, params, {.token = pc->deps_scope.token()}, suspect); + // The send deliberately does NOT run under the supersede scope: the + // master must observe the request's real outcome — the crash + // accounting below depends on it (a wire cancel racing a worker + // death would resume with RequestCancelled and the death would + // dodge the document's ledger). A supersede interrupts the worker + // with a CancelCompile notification instead, and the stale reply is + // discarded at the generation gate below. + auto result = co_await pool.send_stateful(pid, params, {}, suspect); // Crash accounting runs even for superseded compiles: the crash came // from content this document dispatched, and skipping it would let a @@ -1185,11 +1189,11 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { auto pending = session->compiling; if(pending->generation != session->generation) { // The in-flight compile is stale (user edited since it started): - // supersede it. Its dependency waits and its worker send both - // run under deps_scope, so the cancel releases the module-graph - // interest AND wire-cancels the dispatched compile — the worker - // stops the stale parse at the next declaration instead of - // finishing an AST nobody will read. + // supersede it. The launch below interrupts the worker's parse + // with a CancelCompile notification and cancels deps_scope to + // release the module-graph interest; the round itself still runs + // to its (incomplete) reply so crash accounting sees the real + // outcome. break; } co_await pending->done.wait(); @@ -1221,6 +1225,17 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { LOG_INFO("ensure_compiled: launching compile path_id={} gen={}", path_id, session->generation); + if(superseded) { + // Interrupt the stale parse before the replacement can enter the + // pipe: FIFO order guarantees the cancel reaches the worker ahead + // of the new Compile request, so it can only ever hit the stale + // round's stop flag. The request itself is not wire-cancelled — its + // reply (incomplete, or a crash) still reaches run_compile. + pool.notify_stateful( + path_id, + worker::CancelCompileParams{std::string(workspace.path_pool.resolve(path_id))}); + } + // Spawn the replacement before cancelling the superseded compile: the new // round acquires its module-dependency interest synchronously, so shared // dependencies never see their interest drop to zero across the swap. diff --git a/src/server/protocol/worker.h b/src/server/protocol/worker.h index 9cc827365..5cc668f98 100644 --- a/src/server/protocol/worker.h +++ b/src/server/protocol/worker.h @@ -220,6 +220,17 @@ struct EvictedParams { std::string path; }; +/// Interrupt the in-flight compile of `path`, if any. Sent at the master's +/// supersede point instead of wire-cancelling the compile request: the +/// worker flips the compile's stop flag so clang abandons the stale parse +/// at the next declaration, while the request still runs to a normal +/// (incomplete) reply — the master keeps observing the real outcome, so a +/// worker death during a superseded compile still reaches the document's +/// quarantine accounting. +struct CancelCompileParams { + std::string path; +}; + } // namespace clice::worker namespace kota::ipc::protocol { @@ -258,4 +269,9 @@ struct NotificationTraits { constexpr inline static std::string_view method = "clice/worker/evicted"; }; +template <> +struct NotificationTraits { + constexpr inline static std::string_view method = "clice/worker/cancelCompile"; +}; + } // namespace kota::ipc::protocol diff --git a/src/server/state/session.h b/src/server/state/session.h index 48133629c..4895c62aa 100644 --- a/src/server/state/session.h +++ b/src/server/state/session.h @@ -120,10 +120,12 @@ struct Session { /// Generation snapshot at spawn; a later didChange supersedes this compile. std::uint64_t generation = 0; - /// Cancels this round when it is superseded: the module-dependency - /// waits release their interest in the old dependency set, and the - /// worker send (which runs under the same scope) is wire-cancelled - /// so the worker stops the stale parse at the next declaration. + /// Cancels this round's dependency waits when it is superseded, + /// releasing their interest in the old dependency set. The worker + /// send is deliberately not under this scope — the supersede point + /// interrupts the worker's parse with a CancelCompile notification + /// instead, so the round still observes its real outcome (crash + /// accounting depends on it). kota::cancellation_source deps_scope; }; diff --git a/src/server/worker/stateful_worker.cpp b/src/server/worker/stateful_worker.cpp index 72adb0376..b407a4948 100644 --- a/src/server/worker/stateful_worker.cpp +++ b/src/server/worker/stateful_worker.cpp @@ -45,6 +45,14 @@ struct DocumentEntry { // Per-document serialization mutex kota::mutex strand; + + // Stop flag of the most recently arrived Compile request, published + // before its strand wait so a CancelCompile aimed at a queued compile + // still lands. Loop-thread only (the compile closure reads its own + // copy). Never cleared: the master orders CancelCompile ahead of the + // replacement Compile on the pipe, so a set can only ever hit the + // stale round's flag. + std::shared_ptr compile_stop; }; /// RAII ownership of a locked strand. kotatsu cancellation destroys a @@ -163,6 +171,13 @@ void StatefulWorker::register_handlers() { auto doc = get_or_create(params.path); touch_lru(params.path); + // Publish the stop flag before the strand wait: a CancelCompile for + // this round must land even while an earlier request still holds + // the strand — the preset flag then aborts the parse at its first + // declaration. + auto stop = std::make_shared(false); + doc->compile_stop = stop; + co_await doc->strand.lock(); // Every exit — including a cancellation that destroys this frame at @@ -201,14 +216,14 @@ void StatefulWorker::register_handlers() { doc->has_ast = false; doc->unit = CompilationUnit(nullptr); - // The parse itself is cancellable: CompilationParams::stop is - // polled after every top-level declaration, so the hook reaches - // into the middle of the AST build instead of waiting for it to - // finish; an interrupted unit reports !completed() and the phases - // after it are skipped like any other incomplete compile. The - // document stays coherent at every early exit (unit and has_ast - // are set together). - auto stop = std::make_shared(false); + // The parse itself is interruptible: CompilationParams::stop is + // polled after every top-level declaration, so both a CancelCompile + // notification and the queue hook (fired when this frame is + // cancelled) reach into the middle of the AST build instead of + // waiting for it to finish; an interrupted unit reports + // !completed() and the phases after it are skipped like any other + // incomplete compile. The document stays coherent at every early + // exit (unit and has_ast are set together). auto compile_result = co_await kota::queue( [&]() -> worker::CompileResult { ScopedTimer timer; @@ -275,6 +290,15 @@ void StatefulWorker::register_handlers() { [&](DocumentEntry& doc) { return feature::document_links(doc.unit); }); }); + // === CancelCompile === + peer.on_notification([this](const worker::CancelCompileParams& params) { + LOG_DEBUG("CancelCompile notification: path={}", params.path); + auto it = documents.find(params.path); + if(it != documents.end() && it->second->compile_stop) { + it->second->compile_stop->store(true, std::memory_order_relaxed); + } + }); + // === Evict === peer.on_notification([this](const worker::EvictParams& params) { LOG_DEBUG("Evict notification: path={}", params.path); From 960f003027e223abd72f7dd3c84d76219688d67b Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 12:33:08 +0800 Subject: [PATCH 05/12] docs(server): document token forwarding on all forwards --- src/server/compiler/compiler.h | 7 +++++-- src/server/service/feature_router.h | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 009b1496c..344114c98 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -87,7 +87,8 @@ class Compiler { std::optional token = {}); /// Forward a build request (signature help, etc.) to a stateless worker. - /// Sends the full buffer content and compile arguments. + /// Sends the full buffer content and compile arguments. `token`: see + /// forward_query. RawResult forward_build(worker::BuildKind kind, const protocol::Position& position, std::shared_ptr session, @@ -96,11 +97,13 @@ class Compiler { /// Forward a document-link query to the stateful worker holding this /// file's AST. Covers the main-file region only: the preamble's links /// live in the PCH's PreambleState blob (see PCHState::load_state). + /// `token`: see forward_query. kota::task, kota::ipc::Error> forward_document_links(std::shared_ptr session, std::optional token = {}); - /// Forward a formatting request to a stateless worker. + /// Forward a formatting request to a stateless worker. `token`: see + /// forward_query. RawResult forward_format(std::shared_ptr session, std::optional range = {}, std::optional token = {}); diff --git a/src/server/service/feature_router.h b/src/server/service/feature_router.h index a77b13e79..334e86004 100644 --- a/src/server/service/feature_router.h +++ b/src/server/service/feature_router.h @@ -65,6 +65,8 @@ class FeatureRouter { /// targets, the index, and the worker's AST, with an index/directive /// retry after the forward's compile refreshes a dirty session. /// @param session may be null (document not open). + /// @param token the request's cancellation token, forwarded to the + /// worker sends (see Compiler::forward_query). RawResult definition(std::shared_ptr session, llvm::StringRef path, const protocol::Position& pos, @@ -78,6 +80,8 @@ class FeatureRouter { /// than one source, and these are the designated hooks for a future /// read-only provider strategy (e.g. serving closed files from the index). /// They are NOT dead code to be inlined back into the transports. + /// Each takes the request's cancellation token and forwards it to the + /// worker sends (see Compiler::forward_query). RawResult hover(std::shared_ptr session, const protocol::Position& position, std::optional token = {}); From c294a3b30d51adbcec2ca33376cea98be592f3a0 Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 12:33:08 +0800 Subject: [PATCH 06/12] test(server): pin worker interrupt and shared compile --- tests/unit/server/cancel_chain_tests.cpp | 8 +- tests/unit/server/compiler_tests.cpp | 102 +++++++++++++++++++- tests/unit/server/stateful_worker_tests.cpp | 57 +++++++++++ 3 files changed, 159 insertions(+), 8 deletions(-) diff --git a/tests/unit/server/cancel_chain_tests.cpp b/tests/unit/server/cancel_chain_tests.cpp index 933f175a6..8398ad6b9 100644 --- a/tests/unit/server/cancel_chain_tests.cpp +++ b/tests/unit/server/cancel_chain_tests.cpp @@ -84,9 +84,11 @@ TEST_CASE(HandlerCancelChainsThrough) { }); ASSERT_TRUE(test_done); - // Encode the observation in assertions so the outcome is unambiguous: - // handler_resumed distinguishes resume-with-error from frame teardown. - EXPECT_TRUE(handler_resumed || !observed_cancelled_reply); + // The resumption-boundary claim itself: the send must RESUME with a + // cancelled error (emitting the wire cancel on the way), not be torn + // down by the handler's cancellation cascade. + EXPECT_TRUE(handler_resumed); + EXPECT_TRUE(observed_cancelled_reply); } }; // TEST_SUITE(CancelChain) diff --git a/tests/unit/server/compiler_tests.cpp b/tests/unit/server/compiler_tests.cpp index 8b3079cb6..1571ed92a 100644 --- a/tests/unit/server/compiler_tests.cpp +++ b/tests/unit/server/compiler_tests.cpp @@ -358,11 +358,12 @@ TEST_CASE(StopUnblocksCompileWaiters) { } TEST_CASE(SupersededCompileCancelled) { - // An edit mid-compile supersedes the in-flight round entirely: the - // waiter breaks out, the stale round's scope cancels its worker send - // (wire-cancelling the parse), and the replacement compiles the new - // content. Pre-#509 the stale round was left to finish because the - // send was not cancellable. + // An edit mid-compile supersedes the in-flight round: the waiter breaks + // out, the supersede point interrupts the worker's parse with a + // CancelCompile notification, and the replacement compiles the new + // content. This pins the supersede path's liveness (both waiters + // resolve, the fresh AST lands); the interruption itself is pinned + // content-wise by StatefulWorker.CancelNotificationInterruptsCompile. logging::set_anomaly_trap_for_testing([](logging::AnomalyId) {}); TempDir tmp; @@ -441,6 +442,97 @@ TEST_CASE(SupersededCompileCancelled) { logging::reset_anomaly_for_testing(); } +TEST_CASE(ClientCancelSparesCompile) { + // A client's $/cancelRequest tears down one request's frame, never the + // shared compile it waits on: the detached round serves every waiter. + // A regression that threads the request token into the shared round + // would kill waiter B's result along with waiter A's frame. + logging::set_anomaly_trap_for_testing([](logging::AnomalyId) {}); + + TempDir tmp; + tmp.touch("shared.cpp", ""); + auto src = tmp.path("shared.cpp"); + + kota::event_loop loop; + Workspace workspace; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + + auto session = std::make_shared(); + session->path_id = workspace.path_pool.intern(src); + std::string text; + text.reserve(1 << 21); + for(int i = 0; i < 50'000; ++i) { + text += std::format("int v{};\n", i); + } + session->text = std::move(text); + session->line_starts = kota::ipc::lsp::build_line_starts(session->text); + + bool cancelled_returned = false; + bool other_answered = false; + bool done = false; + auto body = [&]() -> kota::task<> { + WorkerPoolOptions opts; + opts.self_path = clice_binary(); + opts.stateless_count = 0; + opts.stateful_count = 1; + CO_ASSERT_TRUE(pool.start(opts)); + co_await kota::sleep(500); + + kota::cancellation_source source; + kota::task_group<> group(loop); + auto cancelled_waiter = [&]() -> kota::task<> { + auto hover = [&]() -> Compiler::RawResult { + co_return co_await compiler.forward_query(worker::QueryKind::Hover, + session, + protocol::Position{0, 4}, + {}, + source.token()); + }; + [[maybe_unused]] auto r = co_await kota::with_token(hover(), source.token()); + cancelled_returned = true; + }; + auto other_waiter = [&]() -> kota::task<> { + auto result = co_await compiler.forward_query(worker::QueryKind::Hover, + session, + protocol::Position{0, 4}); + other_answered = result.has_value(); + }; + group.spawn(cancelled_waiter()); + group.spawn(other_waiter()); + + for(int i = 0; i < 100 && session->compiling == nullptr; ++i) { + co_await kota::sleep(10); + } + CO_ASSERT_TRUE(session->compiling != nullptr); + source.cancel(); + + for(int i = 0; i < 600 && !other_answered; ++i) { + co_await kota::sleep(100); + } + if(!other_answered) { + group.cancel(); + } + co_await group.join(); + + EXPECT_TRUE(cancelled_returned); + EXPECT_TRUE(other_answered); + EXPECT_FALSE(session->ast_dirty); + EXPECT_TRUE(session->compiling == nullptr); + + co_await compiler.stop(); + co_await pool.stop(); + done = true; + }; + auto task = body(); + loop.schedule(task); + loop.run(); + EXPECT_TRUE(done); + + logging::reset_anomaly_for_testing(); +} + TEST_CASE(PoisonPreambleBudget) { // One document's quarantine cannot contain a poison preamble: the PCH // is shared, so every session with the same preamble would re-trigger diff --git a/tests/unit/server/stateful_worker_tests.cpp b/tests/unit/server/stateful_worker_tests.cpp index 951047daf..9b0975cbd 100644 --- a/tests/unit/server/stateful_worker_tests.cpp +++ b/tests/unit/server/stateful_worker_tests.cpp @@ -126,6 +126,63 @@ TEST_CASE(CancelledCompileFreesStrand) { ASSERT_TRUE(test_done); } +TEST_CASE(CancelNotificationInterruptsCompile) { + TempDir tmp; + tmp.touch("interrupt.cpp", ""); + auto src = tmp.path("interrupt.cpp"); + + WorkerHandle w; + ASSERT_TRUE(w.spawn(4ULL * 1024 * 1024 * 1024)); + + bool test_done = false; + + w.run([&]() -> kota::task<> { + std::string text; + text.reserve(1 << 22); + for(int i = 0; i < 200'000; ++i) { + text += std::format("int v{};\n", i); + } + + worker::CompileParams cp; + cp.path = src; + cp.version = 1; + cp.text = std::move(text); + cp.directory = "/tmp"; + cp.arguments = make_args(src); + cp.pch = {"", 0}; + cp.pcms = {}; + + // The notification chases the request down the pipe and flips the + // stop flag mid-parse. Unlike a wire cancel the request runs to a + // reply, and an interrupted parse reports no deps and no index — + // the reply's content, not the clock, is the assertion. + std::optional reply; + kota::task_group<> group(w.loop); + auto sender = [&]() -> kota::task<> { + kota::ipc::request_options opts; + opts.timeout = std::chrono::milliseconds(30'000); + auto result = co_await w.peer->send_request(cp, opts); + if(result.has_value()) { + reply = std::move(result.value()); + } + }; + group.spawn(sender()); + co_await kota::sleep(20, w.loop); + w.peer->send_notification(worker::CancelCompileParams{src}); + co_await group.join(); + + CO_ASSERT_TRUE(reply.has_value()); + EXPECT_EQ(reply->version, 1); + EXPECT_TRUE(reply->deps.empty()); + EXPECT_TRUE(reply->tu_index_data.empty()); + + test_done = true; + w.peer->close_output(); + }); + + ASSERT_TRUE(test_done); +} + TEST_CASE(CancelledQueryFreesStrand) { TempDir tmp; tmp.touch("query_cancel.cpp", ""); From 5bad4a7c52c57a06e4b095960e003330c01940ad Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 12:33:08 +0800 Subject: [PATCH 07/12] test(integration): assert cancel code, tiny hover --- tests/integration/features/test_cancellation.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/integration/features/test_cancellation.py b/tests/integration/features/test_cancellation.py index 6027d5097..4def094ce 100644 --- a/tests/integration/features/test_cancellation.py +++ b/tests/integration/features/test_cancellation.py @@ -21,7 +21,8 @@ async def test_cancelled_completion_replies(executable, tmp_path): (tmp_path / "slow.cpp").write_text(SLOW) - write_cdb(tmp_path, ["slow.cpp"]) + (tmp_path / "tiny.cpp").write_text("int value = 42;\n") + write_cdb(tmp_path, ["slow.cpp", "tiny.cpp"]) client = await make_client(executable, tmp_path) try: @@ -43,12 +44,17 @@ async def test_cancelled_completion_replies(executable, tmp_path): await asyncio.sleep(0.1) client.protocol.notify("$/cancelRequest", CancelParams(id=msg_id)) - # The cancel must produce a prompt error reply, not a full compile. - with pytest.raises(Exception): + # The reply must be the LSP RequestCancelled error (-32800), not a + # timeout or a transport failure passing for one. + with pytest.raises(Exception) as exc: await asyncio.wait_for(task, timeout=30) + assert getattr(exc.value, "code", None) == -32800 - # The server survives the cancellation and still answers. - hover = await client.hover_at(uri, 0, 5) + # The server survives the cancellation and still answers. Hover a + # small file: the slow one would recompile from scratch here and + # can brush the 30s client timeout on a debug CI runner. + tiny_uri, _ = client.open(tmp_path / "tiny.cpp") + hover = await client.hover_at(tiny_uri, 0, 5) assert hover is not None finally: await shutdown_client(client) From 17326b3ae7eac3426b084a6071b86c3e4db53ac2 Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 13:10:09 +0800 Subject: [PATCH 08/12] fix(server): interrupt stale compile on edit codex review: the CancelCompile notification only fired when a later ensure_compiled observed the stale round; an edit with no follow-up request left the stale parse running and its waiters blocked. Single emission point Compiler::interrupt_superseded, called from didChange and the supersede point. --- src/server/compiler/compiler.cpp | 30 +++++--- src/server/compiler/compiler.h | 9 +++ src/server/transport/lsp_client.cpp | 6 ++ tests/unit/server/cancel_chain_tests.cpp | 3 +- tests/unit/server/compiler_tests.cpp | 92 +++++++++++++++++++++++- 5 files changed, 126 insertions(+), 14 deletions(-) diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index 43c2b11bd..ab689dc10 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -1219,23 +1219,18 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { } auto superseded = session->compiling; + + // Interrupt the stale parse before the replacement can enter the pipe: + // FIFO order guarantees the cancel reaches the worker ahead of the new + // Compile request, so it can only ever hit the stale round's stop flag. + interrupt_superseded(*session); + auto pending_compile = std::make_shared(); pending_compile->generation = session->generation; session->compiling = pending_compile; LOG_INFO("ensure_compiled: launching compile path_id={} gen={}", path_id, session->generation); - if(superseded) { - // Interrupt the stale parse before the replacement can enter the - // pipe: FIFO order guarantees the cancel reaches the worker ahead - // of the new Compile request, so it can only ever hit the stale - // round's stop flag. The request itself is not wire-cancelled — its - // reply (incomplete, or a crash) still reaches run_compile. - pool.notify_stateful( - path_id, - worker::CancelCompileParams{std::string(workspace.path_pool.resolve(path_id))}); - } - // Spawn the replacement before cancelling the superseded compile: the new // round acquires its module-dependency interest synchronously, so shared // dependencies never see their interest drop to zero across the swap. @@ -1252,6 +1247,19 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { co_return !session->ast_dirty; } +void Compiler::interrupt_superseded(Session& session) { + if(!session.compiling || session.compiling->generation == session.generation) { + return; + } + // Not a wire cancel: the notification flips the compile's stop flag and + // the request still completes into run_compile's crash accounting (see + // the send site). A stale set is impossible — every emitter runs before + // the replacement Compile can enter the pipe. + pool.notify_stateful( + session.path_id, + worker::CancelCompileParams{std::string(workspace.path_pool.resolve(session.path_id))}); +} + Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, std::shared_ptr session, std::optional position, diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 344114c98..638f7778f 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -68,6 +68,15 @@ class Compiler { /// file_index, pch_key, ast_deps, and publishes diagnostics. kota::task ensure_compiled(std::shared_ptr session); + /// Interrupt the in-flight compile if the buffer moved past it + /// (generation mismatch): the worker abandons the stale parse at the + /// next declaration, while the request still runs to its reply — crash + /// accounting keeps observing the real outcome. Called from the edit + /// path (an edit with no follow-up request must not leave the stale + /// parse holding up its waiters) and from ensure_compiled's supersede + /// point; a no-op when nothing is in flight or the round is current. + void interrupt_superseded(Session& session); + using RawResult = kota::task; /// Forward a query to the stateful worker that holds this file's AST. diff --git a/src/server/transport/lsp_client.cpp b/src/server/transport/lsp_client.cpp index 4b4575028..dc7c16b89 100644 --- a/src/server/transport/lsp_client.cpp +++ b/src/server/transport/lsp_client.cpp @@ -277,6 +277,12 @@ void LSPClient::register_document_sync() { srv.sessions.apply_change(*session, params.content_changes, params.text_document.version); + // The edit just made any in-flight compile stale. Interrupt the + // worker's parse now instead of waiting for the next AST-backed + // request to observe the supersede: with no follow-up request the + // stale parse would run to completion and hold up its waiters. + srv.compiler.interrupt_superseded(*session); + srv.dispatch(FileEvent::buffer_edited(path_id)); LOG_DEBUG("didChange: path={} version={} gen={}", diff --git a/tests/unit/server/cancel_chain_tests.cpp b/tests/unit/server/cancel_chain_tests.cpp index 8398ad6b9..4598c3f25 100644 --- a/tests/unit/server/cancel_chain_tests.cpp +++ b/tests/unit/server/cancel_chain_tests.cpp @@ -55,7 +55,8 @@ TEST_CASE(HandlerCancelChainsThrough) { opts.token = source.token(); auto result = co_await w.peer->send_request(cp, opts); handler_resumed = true; - observed_cancelled_reply = !result.has_value(); + observed_cancelled_reply = + !result.has_value() && result.error().code == worker::dispatch_errc::cancelled; }; kota::task_group<> group(w.loop); diff --git a/tests/unit/server/compiler_tests.cpp b/tests/unit/server/compiler_tests.cpp index 1571ed92a..f170974a8 100644 --- a/tests/unit/server/compiler_tests.cpp +++ b/tests/unit/server/compiler_tests.cpp @@ -357,6 +357,94 @@ TEST_CASE(StopUnblocksCompileWaiters) { logging::reset_anomaly_for_testing(); } +TEST_CASE(EditInterruptsStaleCompile) { + // The didChange path: an edit with NO follow-up request interrupts the + // in-flight parse via interrupt_superseded. The superseded round's + // waiter resolves false (its result is for a buffer that no longer + // exists — the editor re-requests after an edit) instead of sitting + // behind a stale 200k-declaration parse, and the next request compiles + // the fresh content. Liveness pin; the interruption content is pinned + // by StatefulWorker.CancelNotificationInterruptsCompile. + logging::set_anomaly_trap_for_testing([](logging::AnomalyId) {}); + + TempDir tmp; + tmp.touch("edited_only.cpp", ""); + auto src = tmp.path("edited_only.cpp"); + + kota::event_loop loop; + Workspace workspace; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + + auto session = std::make_shared(); + session->path_id = workspace.path_pool.intern(src); + std::string text; + text.reserve(1 << 22); + for(int i = 0; i < 200'000; ++i) { + text += std::format("int v{};\n", i); + } + session->text = std::move(text); + + bool waiter_done = false; + bool waiter_ok = false; + bool done = false; + auto body = [&]() -> kota::task<> { + WorkerPoolOptions opts; + opts.self_path = clice_binary(); + opts.stateless_count = 0; + opts.stateful_count = 1; + CO_ASSERT_TRUE(pool.start(opts)); + co_await kota::sleep(500); + + kota::task_group<> group(loop); + auto waiter = [&]() -> kota::task<> { + waiter_ok = co_await compiler.ensure_compiled(session); + waiter_done = true; + }; + group.spawn(waiter()); + + for(int i = 0; i < 100 && session->compiling == nullptr; ++i) { + co_await kota::sleep(10); + } + CO_ASSERT_TRUE(session->compiling != nullptr); + + // What the didChange handler does: fold the edit in, then interrupt. + session->text = "int fixed;\n"; + session->generation += 1; + session->ast_dirty = true; + compiler.interrupt_superseded(*session); + + for(int i = 0; i < 600 && !waiter_done; ++i) { + co_await kota::sleep(100); + } + if(!waiter_done) { + group.cancel(); + } + co_await group.join(); + + CO_ASSERT_TRUE(waiter_done); + EXPECT_FALSE(waiter_ok); + EXPECT_TRUE(session->compiling == nullptr); + + // The next request (the editor re-queries after an edit) compiles + // the fresh content. + bool second_ok = co_await compiler.ensure_compiled(session); + EXPECT_TRUE(second_ok); + EXPECT_FALSE(session->ast_dirty); + + co_await compiler.stop(); + co_await pool.stop(); + done = true; + }; + auto task = body(); + loop.schedule(task); + loop.run(); + EXPECT_TRUE(done); + + logging::reset_anomaly_for_testing(); +} + TEST_CASE(SupersededCompileCancelled) { // An edit mid-compile supersedes the in-flight round: the waiter breaks // out, the supersede point interrupts the worker's parse with a @@ -490,8 +578,8 @@ TEST_CASE(ClientCancelSparesCompile) { {}, source.token()); }; - [[maybe_unused]] auto r = co_await kota::with_token(hover(), source.token()); - cancelled_returned = true; + auto r = co_await kota::with_token(hover(), source.token()); + cancelled_returned = r.is_cancelled(); }; auto other_waiter = [&]() -> kota::task<> { auto result = co_await compiler.forward_query(worker::QueryKind::Hover, From 913273b3a703f37571d76f7bc82fadbae547fa87 Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 13:45:48 +0800 Subject: [PATCH 09/12] fix(server): abandon superseded round fully on edit codex round 2: an edit landing while the round is in dependency prep only sent CancelCompile (a worker no-op pre-dispatch); the module-graph waits kept the waiters blocked. abandon_superseded = interrupt + deps scope cancel, for the edit path where no replacement round follows. Also yield before the synchronous include-completion scan so a piped $/cancelRequest tears the frame before the directory walk. --- src/server/compiler/compiler.cpp | 14 ++++++++++++ src/server/compiler/compiler.h | 16 +++++++++---- src/server/service/feature_router.cpp | 7 ++++++ src/server/transport/lsp_client.cpp | 10 ++++---- tests/unit/server/compiler_tests.cpp | 33 +++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index ab689dc10..3ccf9e670 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -1260,6 +1260,20 @@ void Compiler::interrupt_superseded(Session& session) { worker::CancelCompileParams{std::string(workspace.path_pool.resolve(session.path_id))}); } +void Compiler::abandon_superseded(Session& session) { + if(!session.compiling || session.compiling->generation == session.generation) { + return; + } + interrupt_superseded(session); + // The round may still be in dependency prep, where the notification + // cannot reach it (nothing dispatched yet): cancel its waits so it + // unwinds now instead of after the module graph settles. The cancel + // cascade can finish the round synchronously — session.compiling may + // be null when this returns. (A round inside its PCH build stays until + // the shared build replies: that send is deliberately scope-free.) + session.compiling->deps_scope.cancel(); +} + Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, std::shared_ptr session, std::optional position, diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 638f7778f..f42b6402d 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -71,12 +71,20 @@ class Compiler { /// Interrupt the in-flight compile if the buffer moved past it /// (generation mismatch): the worker abandons the stale parse at the /// next declaration, while the request still runs to its reply — crash - /// accounting keeps observing the real outcome. Called from the edit - /// path (an edit with no follow-up request must not leave the stale - /// parse holding up its waiters) and from ensure_compiled's supersede - /// point; a no-op when nothing is in flight or the round is current. + /// accounting keeps observing the real outcome. Deliberately does NOT + /// touch deps_scope: the supersede point orders that cancel after the + /// replacement spawn so module interest never dips to zero across the + /// swap. A no-op when nothing is in flight or the round is current. void interrupt_superseded(Session& session); + /// The edit path's whole supersede: interrupt the worker's parse AND + /// cancel the stale round's dependency waits. With no replacement + /// round coming there is no interest hand-off to order against, and a + /// round parked in dependency prep (module graph waits) would + /// otherwise hold its waiters until the graph settles. A no-op when + /// nothing is in flight or the round is current. + void abandon_superseded(Session& session); + using RawResult = kota::task; /// Forward a query to the stateful worker that holds this file's AST. diff --git a/src/server/service/feature_router.cpp b/src/server/service/feature_router.cpp index fee958c94..4826719cf 100644 --- a/src/server/service/feature_router.cpp +++ b/src/server/service/feature_router.cpp @@ -247,6 +247,13 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr sess auto pctx = detect_completion_context(session->text, *offset); if(pctx.kind == CompletionContext::IncludeQuoted || pctx.kind == CompletionContext::IncludeAngled) { + // The include scan is synchronous and this handler is resumed + // eagerly, so a $/cancelRequest sitting in the pipe (rapid-fire + // completions cancel their predecessors) has not been read yet. + // Yield once: the loop drains the pipe, and a fired token tears + // this frame down at the suspension instead of paying for a + // directory walk nobody wants. + co_await kota::yield(); std::string directory; std::vector arguments; contexts.resolve_command(path, directory, arguments); diff --git a/src/server/transport/lsp_client.cpp b/src/server/transport/lsp_client.cpp index dc7c16b89..0ea37dda6 100644 --- a/src/server/transport/lsp_client.cpp +++ b/src/server/transport/lsp_client.cpp @@ -277,11 +277,11 @@ void LSPClient::register_document_sync() { srv.sessions.apply_change(*session, params.content_changes, params.text_document.version); - // The edit just made any in-flight compile stale. Interrupt the - // worker's parse now instead of waiting for the next AST-backed - // request to observe the supersede: with no follow-up request the - // stale parse would run to completion and hold up its waiters. - srv.compiler.interrupt_superseded(*session); + // The edit just made any in-flight compile stale. Abandon it now + // instead of waiting for the next AST-backed request to observe + // the supersede: with no follow-up request the stale parse (or its + // dependency prep) would run to completion and hold up its waiters. + srv.compiler.abandon_superseded(*session); srv.dispatch(FileEvent::buffer_edited(path_id)); diff --git a/tests/unit/server/compiler_tests.cpp b/tests/unit/server/compiler_tests.cpp index f170974a8..96e85836b 100644 --- a/tests/unit/server/compiler_tests.cpp +++ b/tests/unit/server/compiler_tests.cpp @@ -357,6 +357,39 @@ TEST_CASE(StopUnblocksCompileWaiters) { logging::reset_anomaly_for_testing(); } +TEST_CASE(AbandonCancelsDepsScope) { + // The two supersede entry points differ on deps_scope by design: + // abandon_superseded (the edit path, no replacement round) cancels the + // stale round's dependency waits; interrupt_superseded (the supersede + // point) must not — its deps cancel is ordered after the replacement + // spawn so module interest never dips to zero across the swap. + TempDir tmp; + tmp.touch("scoped.cpp", ""); + + kota::event_loop loop; + Workspace workspace; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + + auto session = std::make_shared(); + session->path_id = workspace.path_pool.intern(tmp.path("scoped.cpp")); + session->compiling = std::make_shared(); + session->compiling->generation = session->generation; + + // Current round: both are no-ops. + compiler.abandon_superseded(*session); + EXPECT_FALSE(session->compiling->deps_scope.cancelled()); + + session->generation += 1; + + compiler.interrupt_superseded(*session); + EXPECT_FALSE(session->compiling->deps_scope.cancelled()); + + compiler.abandon_superseded(*session); + EXPECT_TRUE(session->compiling->deps_scope.cancelled()); +} + TEST_CASE(EditInterruptsStaleCompile) { // The didChange path: an edit with NO follow-up request interrupts the // in-flight parse via interrupt_superseded. The superseded round's From daace12f9abc0e4c14eeb442cf4e8bcfcd5d04cb Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 14:17:08 +0800 Subject: [PATCH 10/12] fix(completion): read buffer state after the yield codex round 3: the yield left pctx/offset computed from the pre-edit buffer; a didChange landing during the suspension made the include scan serve candidates and TextEdit ranges for text that no longer exists. Move the suspension before every buffer read: the synchronous remainder serves one consistent snapshot, and every completion path gains the early-cancel window. --- src/server/service/feature_router.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/server/service/feature_router.cpp b/src/server/service/feature_router.cpp index 4826719cf..51083746d 100644 --- a/src/server/service/feature_router.cpp +++ b/src/server/service/feature_router.cpp @@ -238,6 +238,15 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr sess std::optional token) { auto pause = indexer.scoped_pause(); + // This handler is resumed eagerly, so a $/cancelRequest or didChange + // sitting in the pipe (rapid-fire completions cancel and re-issue as + // the user types) has not been read yet. Yield once BEFORE reading any + // buffer state: the loop drains the pipe — a fired token tears this + // frame down here, and an edit lands before the offset and completion + // context are computed, so the synchronous include scan below never + // serves candidates or ranges for a buffer that no longer exists. + co_await kota::yield(); + auto path_id = session->path_id; auto path = std::string(workspace.path_pool.resolve(path_id)); @@ -247,13 +256,6 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr sess auto pctx = detect_completion_context(session->text, *offset); if(pctx.kind == CompletionContext::IncludeQuoted || pctx.kind == CompletionContext::IncludeAngled) { - // The include scan is synchronous and this handler is resumed - // eagerly, so a $/cancelRequest sitting in the pipe (rapid-fire - // completions cancel their predecessors) has not been read yet. - // Yield once: the loop drains the pipe, and a fired token tears - // this frame down at the suspension instead of paying for a - // directory walk nobody wants. - co_await kota::yield(); std::string directory; std::vector arguments; contexts.resolve_command(path, directory, arguments); From ee40a2ba6ac1b2a1ef47d7e0cea12920101c609f Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 19:09:53 +0800 Subject: [PATCH 11/12] test(integration): e2e cancel across all features --- .../integration/features/test_cancellation.py | 175 ++++++++++++++++-- 1 file changed, 158 insertions(+), 17 deletions(-) diff --git a/tests/integration/features/test_cancellation.py b/tests/integration/features/test_cancellation.py index 4def094ce..802a687bc 100644 --- a/tests/integration/features/test_cancellation.py +++ b/tests/integration/features/test_cancellation.py @@ -6,17 +6,46 @@ import pytest from lsprotocol.types import ( CancelParams, + CodeActionContext, + CodeActionParams, CompletionParams, + DefinitionParams, + DocumentFormattingParams, + DocumentLinkParams, + DocumentRangeFormattingParams, + DocumentSymbolParams, + FoldingRangeParams, + FormattingOptions, + HoverParams, + InlayHintParams, Position, + Range, + SemanticTokensParams, + SignatureHelpParams, TextDocumentIdentifier, ) from tests.tools.compile_commands import write_cdb from tests.tools.lifecycle import make_client, shutdown_client +from tests.tools.workspace import did_change # Two hundred thousand trivial declarations: slow to parse on any hardware, # cheap to abandon (the worker polls the stop flag per declaration). SLOW = "\n".join(f"int v{i};" for i in range(200_000)) + "\n" +LAST_LINE = 199_999 + + +async def cancel_and_expect(client, method, params, timeout=30): + """Send `method`, cancel it 0.1s later, require a RequestCancelled reply.""" + msg_id = str(uuid.uuid4()) + task = asyncio.ensure_future( + client.protocol.send_request_async(method, params, msg_id=msg_id) + ) + await asyncio.sleep(0.1) + client.protocol.notify("$/cancelRequest", CancelParams(id=msg_id)) + with pytest.raises(Exception) as exc: + await asyncio.wait_for(task, timeout=timeout) + assert getattr(exc.value, "code", None) == -32800 async def test_cancelled_completion_replies(executable, tmp_path): @@ -31,24 +60,14 @@ async def test_cancelled_completion_replies(executable, tmp_path): # Complete at the LAST line: clang truncates the parse at the # completion point, so a point at the top would skip the slow body # entirely and the request could finish before the cancel arrives. - params = CompletionParams( - text_document=TextDocumentIdentifier(uri=uri), - position=Position(line=199_999, character=4), - ) - msg_id = str(uuid.uuid4()) - task = asyncio.ensure_future( - client.protocol.send_request_async( - "textDocument/completion", params, msg_id=msg_id - ) + await cancel_and_expect( + client, + "textDocument/completion", + CompletionParams( + text_document=TextDocumentIdentifier(uri=uri), + position=Position(line=LAST_LINE, character=4), + ), ) - await asyncio.sleep(0.1) - client.protocol.notify("$/cancelRequest", CancelParams(id=msg_id)) - - # The reply must be the LSP RequestCancelled error (-32800), not a - # timeout or a transport failure passing for one. - with pytest.raises(Exception) as exc: - await asyncio.wait_for(task, timeout=30) - assert getattr(exc.value, "code", None) == -32800 # The server survives the cancellation and still answers. Hover a # small file: the slow one would recompile from scratch here and @@ -58,3 +77,125 @@ async def test_cancelled_completion_replies(executable, tmp_path): assert hover is not None finally: await shutdown_client(client) + + +async def test_cancelled_signature_help(executable, tmp_path): + # Same shape as completion (the other stateless build kind): the call + # site sits past the slow body, so reaching it means a full parse. + text = SLOW + "void take(int a, int b);\nint use() { return take(1, 2); }\n" + (tmp_path / "sig.cpp").write_text(text) + (tmp_path / "tiny.cpp").write_text("int value = 42;\n") + write_cdb(tmp_path, ["sig.cpp", "tiny.cpp"]) + + client = await make_client(executable, tmp_path) + try: + uri, _ = client.open(tmp_path / "sig.cpp") + + await cancel_and_expect( + client, + "textDocument/signatureHelp", + SignatureHelpParams( + text_document=TextDocumentIdentifier(uri=uri), + position=Position(line=LAST_LINE + 2, character=26), + ), + ) + + tiny_uri, _ = client.open(tmp_path / "tiny.cpp") + hover = await client.hover_at(tiny_uri, 0, 5) + assert hover is not None + finally: + await shutdown_client(client) + + +async def test_cancelled_requests_while_compiling(executable, tmp_path): + # Every forwarded feature cancelled while the document's compile is + # still churning through the slow body. All of them must reply with + # RequestCancelled, and none of them may tear down the shared compile: + # the closing hover on the SAME file must land with the AST it built. + (tmp_path / "slow.cpp").write_text(SLOW) + write_cdb(tmp_path, ["slow.cpp"]) + + client = await make_client(executable, tmp_path) + try: + uri, _ = client.open(tmp_path / "slow.cpp") + doc = TextDocumentIdentifier(uri=uri) + head = Range( + start=Position(line=0, character=0), end=Position(line=10, character=0) + ) + fmt = FormattingOptions(tab_size=4, insert_spaces=True) + + requests = [ + ( + "textDocument/hover", + HoverParams(text_document=doc, position=Position(line=0, character=4)), + ), + ( + "textDocument/definition", + DefinitionParams( + text_document=doc, position=Position(line=0, character=4) + ), + ), + ("textDocument/documentSymbol", DocumentSymbolParams(text_document=doc)), + ( + "textDocument/semanticTokens/full", + SemanticTokensParams(text_document=doc), + ), + ("textDocument/foldingRange", FoldingRangeParams(text_document=doc)), + ("textDocument/inlayHint", InlayHintParams(text_document=doc, range=head)), + ( + "textDocument/codeAction", + CodeActionParams( + text_document=doc, + range=head, + context=CodeActionContext(diagnostics=[]), + ), + ), + ("textDocument/documentLink", DocumentLinkParams(text_document=doc)), + ( + "textDocument/formatting", + DocumentFormattingParams(text_document=doc, options=fmt), + ), + ( + "textDocument/rangeFormatting", + DocumentRangeFormattingParams( + text_document=doc, range=head, options=fmt + ), + ), + ] + for method, params in requests: + await cancel_and_expect(client, method, params) + + hover = await client.hover_at(uri, 0, 4, timeout=120) + assert hover is not None + finally: + await shutdown_client(client) + + +async def test_edit_supersedes_compile(executable, tmp_path): + # An edit mid-compile abandons the stale parse end-to-end: the request + # that launched it resolves promptly (null — the editor re-queries + # after an edit), and the next request answers on the new content. + (tmp_path / "edited.cpp").write_text(SLOW) + write_cdb(tmp_path, ["edited.cpp"]) + + client = await make_client(executable, tmp_path) + try: + uri, _ = client.open(tmp_path / "edited.cpp") + + first = asyncio.ensure_future( + client.text_document_hover_async( + HoverParams( + text_document=TextDocumentIdentifier(uri=uri), + position=Position(line=0, character=4), + ) + ) + ) + await asyncio.sleep(0.3) + did_change(client, uri, version=1, text="int fixed;\n") + + assert await asyncio.wait_for(first, timeout=10) is None + + hover = await client.hover_at(uri, 0, 4) + assert hover is not None + finally: + await shutdown_client(client) From 8b2b945b057fed9469e6e23c5b7e8eaa108d280d Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 16 Jul 2026 19:37:17 +0800 Subject: [PATCH 12/12] fix(tests): fresh compile per cancelled request macOS arm64 finished the shared 200k parse mid-sweep, so the seventh cancel found a ready AST and got a normal reply. Each pulling request now edits first and launches its own parse: the cancel window is per round, not cumulative. --- .../integration/features/test_cancellation.py | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/tests/integration/features/test_cancellation.py b/tests/integration/features/test_cancellation.py index 802a687bc..e4511526d 100644 --- a/tests/integration/features/test_cancellation.py +++ b/tests/integration/features/test_cancellation.py @@ -108,10 +108,11 @@ async def test_cancelled_signature_help(executable, tmp_path): async def test_cancelled_requests_while_compiling(executable, tmp_path): - # Every forwarded feature cancelled while the document's compile is - # still churning through the slow body. All of them must reply with - # RequestCancelled, and none of them may tear down the shared compile: - # the closing hover on the SAME file must land with the AST it built. + # Every forwarded feature cancelled while the compile it waits on is + # still churning through the slow body. Each AST-backed request is + # preceded by an edit, so it launches a FRESH 200k-declaration parse + # and the 0.1s cancel window never depends on how much of a previous + # parse is left (a fast runner finished the shared parse mid-sweep). (tmp_path / "slow.cpp").write_text(SLOW) write_cdb(tmp_path, ["slow.cpp"]) @@ -124,7 +125,7 @@ async def test_cancelled_requests_while_compiling(executable, tmp_path): ) fmt = FormattingOptions(tab_size=4, insert_spaces=True) - requests = [ + pulling = [ ( "textDocument/hover", HoverParams(text_document=doc, position=Position(line=0, character=4)), @@ -151,20 +152,27 @@ async def test_cancelled_requests_while_compiling(executable, tmp_path): ), ), ("textDocument/documentLink", DocumentLinkParams(text_document=doc)), - ( - "textDocument/formatting", - DocumentFormattingParams(text_document=doc, options=fmt), - ), - ( - "textDocument/rangeFormatting", - DocumentRangeFormattingParams( - text_document=doc, range=head, options=fmt - ), - ), ] - for method, params in requests: + version = 0 + for method, params in pulling: + version += 1 + did_change(client, uri, version, SLOW + f"int extra{version};\n") await cancel_and_expect(client, method, params) + # The format pair never pulls an AST, so no edit: the last pulling + # request's compile must survive these cancels too and serve the + # closing hover — the shared compile outlives every client cancel. + await cancel_and_expect( + client, + "textDocument/formatting", + DocumentFormattingParams(text_document=doc, options=fmt), + ) + await cancel_and_expect( + client, + "textDocument/rangeFormatting", + DocumentRangeFormattingParams(text_document=doc, range=head, options=fmt), + ) + hover = await client.hover_at(uri, 0, 4, timeout=120) assert hover is not None finally: