Otherwise, the body task might finish before we yield the `TimeoutError` and thus `withTimeout` would not actually throw a `TimeoutError`.
rdar://137171114
When removing the LanguageServerProtocol namespace from types in
ClientCapabilities (#1734), some types inadvertently started shadowing
themselves. This lead to client initialization errors when the
initialization request couldn't be decoded:
```
type mismatch at params.capabilities.textDocument.completion.completionItemKind.valueSet.Index 0 : Expected to decode Dictionary<String, Any> but found number instead.
```
Disambiguating these inner types in ClientCapabilities fixes decoding.
Otherwise, we can have a retain cycle where the `OutstandingRequest` keeps the `JSONRPCConnection` alive.
Also move the code that handled `outstandingRequests` out of the `Task` because `outstandingRequests` should only be accessed from `queue`.
I saw a nondeterministic test failure of `AsyncUtilsTests.testWithTimeoutEscalatesPriority` and I believe this was the cause.
Also refactor a few test helper functions on the way.
Previously, when we hit the timeout in `withTimeout`, we just cancelled `body` but relied on cooperative cancellation before the caller could continue. Since the caller is obviously no longer interested in the operation, we can return with `CancellationError` immediately.
Fixes#883
rdar://116705684
Showing the list of active requests didn’t turn out to be very useful for debugging. What is more useful, however, is to visualize the requests as the get enqueued and handled in a trace chart. This gives a very intuitive view of which request is blocking another request from getting executed.
The build server is automatically shut down using a background task when `BuildSystemManager` is deallocated.
This, however, leads to possible race conditions where the shutdown task might not finish before the test is done, which could result in the connection being reported as a leak. To avoid this problem, we want to explicitly shut, down the build server when the `SourceKitLSPServer` gets shut down.
The original idea behind this test was that only opening `Crash.swift` would retrieve build settings for `Crash.swift`. But that wasn’t actually true: Scanning for tests also retrieved build settings (to get the test’s module name). This caused test failures if we set `SOURCEKIT_LSP_TEST_TIMEOUT: 10`, like we do in `settings.json`.
Improve the test to actually check that the BSP server has crashed and increase timeout to be bigger than the 10s restart delay.
Adding an item to `AsyncQueue<Serial>` is linear in the number of pending queue items, thus adding n items to an `AsyncQueue` before any can execute is in O(n^2). This decision was made intentionally because the primary use case for `AsyncQueue` was to track pending LSP requests, of which we don’t expect to have too many pending requests at any given time.
`SourceKitIndexDelegate` was also using `AsyncQueue` to track the number of pending units to be processed and eg. after indexing SourceKit-LSP, I have seen this grow up to ~20,000. With the quadratic behavior, this explodes time-wise.
Turns out that we don’t actually need to use a queue here at all, an atomic is sufficient and much faster.
Independently, we should consider mitigating the quadratic behavior of `AsyncQueue<Serial>` or `AsyncQueue` in general.
When we receive build settings after hitting the timeout, we call `fileBuildSettingsChanged` on the delegate, which should cause the document to get re-opened in sourcekitd and diagnostics to get refreshed.
rdar://136332685
If a BSP server crashes, we should try to relaunch it. Use a similar relaunch logic as the one we use for `clangd`: Try restarting immediately unless the BSP server has crashed within the last 30 seconds, in which case we delay the restart by 10s.
rdar://136311009
Fixes#1686
There was a very brief time when a `QueuedTask` would start executing but `isExecuting` was still sent to `false`. This caused the `preparationTask.isExecuting` check in `SemanticIndexManager.prepare` to fail after we did start preparation, causing the preparation of `LibB` in `testDontStackTargetPreparationForEditorFunctionality` to be cancelled, which wasn’t intended. Thus, the expected preparation tracker got out of sync and the test would fail.
rdar://136570212
The interaction to an out-of-process BSP server still went through the `BuildServerBuildSystem`, which doesn’t forward all messages to the build system and uses the old push-based model for build settings.
If we discover that the BSP server supports the new pull-based build settings model, we now forward all methods to it directly, without going through `BuiltInBuildSystemAdapter`, which has been renamed to `LegacyBuildServerBuildSystem`.
rdar://136106323
rdar://127606323
rdar://126493405
Fixes#1226Fixes#1173
