Acquire snapshots synchronously while blocking other requests/notifications

[andrewbranch]

Fixes #2690

Imagine a sequence of

1. textDocument/didChange
2. textDocument/inlayHint
3. textDocument/didChange

When we saw the first change in the server dispatch loop, we would hold processing of the rest of the queue while that change gets pushed. Then we would resume and call s.handleRequestOrNotification with the inlay hint one, and (without waiting for it to complete) handle the next change, holding any more pending requests. The problem is that s.handleRequestOrNotification didn't get passed a snapshot; it looked at the request method, looked up the correct handler in the server, which would then get acquire a snapshot (in the form of a LanguageService) and pass it to the handler. So there was a tiny bit of intermediate time that was available for the next change notification to sneak in.

With this PR, handlers now have a sync portion that gets executed serially in the dispatch loop, which they use to secure a snapshot, and return a function of async work that uses the snapshot.

The downside of this is that if a client isn’t diligent about canceling old requests, we have no choice but to manifest a program for each one, even if we've built up changes that make them stale. The LSP spec says as much:

• if a client sends a request to the server and the client state changes in a way that invalidates the response, the client should do the following:
  • cancel the server request and ignore the result if the result is not useful for the client anymore. If necessary, the client should resend the request.
  • keep the request running if the client can still make use of the result by, for example, transforming it to a new result by applying the state change to the result.
• servers should therefore not decide by themselves to cancel requests simply due to that fact that a state change notification is detected in the queue. As said, the result could still be useful for the client.

So I think this is the right approach.

[jakebailey]

This deletion can now happen before doAsyncWork completes (or even runs), which I think means that requests can't be cancelled.

Probably this needs to be duplicated, or done in an IIFE so it can be deferred?

[jakebailey]

Like:

func() {
	defer func() {
		s.pendingClientRequestsMu.Lock()
		defer s.pendingClientRequestsMu.Unlock()
		delete(s.pendingClientRequests, *req.ID)
	}()

	if doAsyncWork, err := s.handleRequestOrNotification(requestCtx, req); err != nil {
		handleError(err)
	} else if doAsyncWork != nil {
		go func() {
			if lsError := doAsyncWork(); lsError != nil {
				handleError(lsError)
			}
		}()
	}
}()

[DanielRosenwasser]

I do feel like servicing every out-of-date request, especially for the most typical kind of client (e.g. editors) is not ideal - but probably something we can pull back on later.

[Copilot]

Pull request overview

This PR fixes a race condition in the LSP server where snapshots were being acquired asynchronously, allowing file change notifications to be processed before in-flight requests had secured their snapshots. The fix restructures all LSP handlers to use a two-phase pattern: synchronous work (primarily snapshot acquisition) executes in the dispatch loop, blocking other requests, while the actual request processing runs asynchronously in a goroutine using the captured snapshot.

Changes:

• Modified handler signature from func(...) error to func(...) (func() error, error) to separate sync and async work
• Removed isBlockingMethod logic since all handlers now execute sync work in the dispatch loop
• Updated all handler registration functions to support the new two-phase pattern

[andrewbranch]

Sorryyyy