Skip to content

Mark reqnroll project lifecycle notifications as Serial dispatch - #493

Merged
clrudolphi merged 2 commits into
masterfrom
fix/project-lifecycle-messages-serial
Aug 26, 2026
Merged

Mark reqnroll project lifecycle notifications as Serial dispatch#493
clrudolphi merged 2 commits into
masterfrom
fix/project-lifecycle-messages-serial

Conversation

@clrudolphi

Copy link
Copy Markdown
Collaborator

🤔 What's changed?

reqnroll/projectLoaded, reqnroll/projectUnloaded, and reqnroll/projectFiles (in LanguageServerOptionsExtensions.InitializeCustomProtocolRouting) now pass new JsonRpcHandlerOptions { RequestProcessType = RequestProcessType.Serial } to OnNotification, instead of relying on the implicit default. Added LanguageServerOptionsExtensionsTests to assert this at the registration level (and a sanity check that reqnroll/resolveTestTargets — a read-only route — stays off the Serial lane).

⚡️ What's your motivation?

While investigating #491 I traced through OmniSharp's actual request-dispatch resolution (decompiled the library to confirm, rather than going by the docs): this server uses the stock ParallelRequestProcessIdentifier (the library's own default), and every custom reqnroll/* message is registered via manual OnRequest/OnNotification with no JsonRpcHandlerOptions argument. That combination means every custom reqnroll/* handler currently runs Parallel — including the three project lifecycle notifications.

ILspWorkspaceScopeManager's scope table is a ConcurrentDictionary, which protects against concurrent corruption but makes no guarantee about completion order. Under Parallel dispatch, a fast projectUnloaded could finish before a slower, earlier-sent projectLoaded for the same project completes — leaving the scope loaded when the client already believes it's gone (or the reverse, on a rapid solution reload/rename). That's a real correctness hazard for project lifecycle specifically, since these notifications must apply in the order the client sent them.

I deliberately did not extend this to any other manual route. Per the existing rationale in IFeatureParseCoordinator's remarks (issue #471), Serial is not free: an in-flight Serial item blocks the start of new Parallel work too, since it shares one global FIFO lane with didOpen/didChange/didSave. Every other manual route (resolveTestTargets, goToStepDefinitions/Hooks/MatchingScenarios, findStepUsages, findUnusedStepDefinitions, the codeLens family, inlayHint/foldingRange, the rename family) is either a read-only query or already internally synchronized (RenameSessionManager keys sessions by (uri, version) in a ConcurrentDictionary with atomic TryRemove-based consumption) — forcing those onto the Serial lane would only add latency with no correctness benefit.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

🧩 Area(s) touched

  • LSP server (src/LSP)

📋 Checklist:

  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change alters as-built behaviour described in a design doc under docs/
  • Users should know about my change

🤖 Generated with Claude Code

clrudolphi and others added 2 commits August 26, 2026 14:20
reqnroll/projectLoaded, reqnroll/projectUnloaded, and reqnroll/projectFiles
are registered via manual OnNotification with no JsonRpcHandlerOptions,
which defaults to this server's global Parallel dispatch identifier
(JsonRpcServerOptions' default). All three mutate the same
ILspWorkspaceScopeManager scope table via a ConcurrentDictionary, which
guards against concurrent corruption but not completion order: under
Parallel dispatch a fast projectUnloaded could finish before a slower,
earlier-sent projectLoaded for the same project, leaving the scope loaded
when the client already thinks it's gone (or the reverse, on a rapid
solution reload).

Pass RequestProcessType.Serial explicitly for these three so the client's
send order is preserved. Left every other manual route (resolveTestTargets,
goToStepDefinitions/Hooks/MatchingScenarios, findStepUsages,
findUnusedStepDefinitions, codeLens family, inlayHint/foldingRange, rename
family) on the default Parallel lane — they're read-only queries or already
internally synchronized (RenameSessionManager), and per issue #471 forcing
Serial has a real cost: an in-flight Serial item blocks the *start* of new
Parallel work too, on the same shared FIFO lane as didOpen/didChange/didSave.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@clrudolphi
clrudolphi merged commit ad630dd into master Aug 26, 2026
16 checks passed
@clrudolphi
clrudolphi deleted the fix/project-lifecycle-messages-serial branch August 26, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant