For ID counters and standalone flags (most sites), drop to .relaxed
since nothing reads other memory based on the atomic's value.
MultiEntrySemaphore.signaled is the exception: it is used as a
signal/wait primitive where waiters proceed to use state set up
before signal(). Use .releasing on store and .acquiring on load so
that pre-signal writes are visible to waiters that observe `true`.
.relaxed there would be incorrect on weakly-ordered architectures.
Replace AtomicBool/UInt8/UInt32/Int32 from
ToolsProtocolsSwiftExtensions with Synchronization.Atomic<T> where
the storage is a static, module-level let, or class stored property.
For local lets captured by @Sendable closures (where Atomic's
~Copyable nature prevents capture), use ThreadSafeBox<T> instead.
- **`sourcekit/workspace/symbolNames`** — returns a flat, deduplicated
list of every symbol name in the workspace index (source and indexed
system modules). Clients use this to drive their search UI locally.
- **`sourcekit/workspace/symbolInfo`** — given a list of exact symbol
names, returns `WorkspaceSymbolItem` for each occurrence across all
workspaces, for display in the search result list. Source-file symbols
get `SymbolInformation` with a `file://` location. SDK/stdlib symbols
get a `WorkspaceSymbol` with `location: .uri(…)` The client must call
`workspaceSymbol/resolve` after the user selects an SDK/stdlib symbol to
obtain the concrete interface location.
- **`workspaceSymbol/resolve`** — resolves the deferred
`WorkspaceSymbol` location from `sourcekit/workspace/symbolInfo`. Parses
the `?module=` value into `moduleName`/`groupName`, finds a real source
file via `mainFiles(containing:)`, calls `openGeneratedInterface`, and
returns the symbol with `location` replaced by a full
`sourcekit-lsp://generated-swift-interface/` URI + range (or a temp
`file://` path for clients without `workspace/getReferenceDocument`
support).
Instead of querying the index twice (once for up-to-date files, once for
outdated ones), collect file modification timestamps during the syntactic
scan phase and use them to filter a single semantic index query.
For files that don't support syntactic scans, use the semantic index
results even if outdated.
Also extracts symlink-aware mtime resolution into URL.fileModificationDate
and exposes snapshotHasInMemoryModifications(_:) on DocumentManager.
`IndexStoreDB` moves its index to the `saved` directory when it is deallocated. Because `IndexStoreDB` is primarily owned by `UncheckedIndex`, we rely on deallocating this object to save the index store. This is fairly brittle because various parts of the codebase may hold transient references to that object as reported in https://github.com/swiftlang/sourcekit-lsp/issues/2455#issuecomment-3873561003.
Explicitly remove the reference from `UncheckedIndex` to `IndexStoreDB`. While this still isn’t perfect because other parts of the code base may hold references to `IndexStoreDB` but those should be a lot rarer, resulting in a more consistent closing of the index.
Replace usages of `Optional.map` and `Optional.flatMap` by if expressions or other expressions.
I personally find `Optional.map` to be hard to read because `map` implies mapping a collection to me. Usually the alternative constructs seem clearer to me.
The reason why I am making the first change is because in a separate PR in
swiftlang I am fixing a bug that caused certain captured parameters to be
treated as sending parameters incorrectly. This allowed for parameters to
incorrectly be allowed to be sent from one isolation domain to another.
The specific problem here can be seen with the following swift code:
```swift
actor B {
init(callback: @escaping @Sendable () -> Void) async {}
}
actor A {
private func poke() {}
func schedule() async {
_ = await B(
callback: { [weak self] in // closure 1
Task.detached { // closure 2
await self?.poke()
}
})
}
}
```
When we capture the weak self from closure 1 in closure 2, we are not actually
capturing self directly. Instead we are capturing the var box which contains the
weak self. The box (unlike self) is actually non-Sendable. Since closure 2 is
not call(once), the compiler must assume semantically that the closure can be
invoked potentially multiple times meaning that it cannot allow for self to be
used in Task.detached. The fix for this is to perform an inner [weak self]
capture. As follows:
```swift
actor A {
private func poke() {}
func schedule() async {
_ = await B(
callback: { [weak self] in // closure 1
Task.detached { [weak self] // closure 2
await self?.poke()
}
})
}
}
```
The reason why this works is that when we form the second weak self binding, we
perform a load from the outer weak self giving us an Optional<A>. Then we store
that optional value back into a new weak box. Since Optional<A> is Sendable, we
know that the two non-Sendable weak var boxes are completely unrelated, so we
can send that new var box into the new Task.detached safely.
The second `[weak self]` is just something I noticed later in the function. The
`[weak self]` just makes the detached function safer.
Until we have better measurements that would motivate a different batching strategy, copying the driver’s batch size seems like the most reasonable thing to do.
According to https://developer.apple.com/documentation/foundation/processinfo/activeprocessorcount
> Whereas the processorCount property reports the number of advertised processing cores, the activeProcessorCount property reflects the actual number of active processing cores on the system. There are a number of different factors that may cause a core to not be active, including boot arguments, thermal throttling, or a manufacturing defect.
For short-lived workloads like `concurrentMap` we want to parallelize across the number of cores that are currently active, so use `activeProcessorCount` instead. The only case where we want to continue using `processorCount` is the computation of concurrent tasks for `TaskScheduler` because the value is stored for the lifetime of the SourceKit-LSP process and we don’t want to limit parallelism if SourceKit-LSP was launched during a time of thermal throttling.
I stumbled across this while working on #2302
We should not take the number of files in an `UpdateIndexStoreTaskDescription` as an indication on how important the task is. If we do need this functionality, eg. because we want to update the index of files with syntactic matches for a rename term, this should be communicated using a specific purpose similar to `TargetPreparationPurpose`. Since the only reason we update the index store for a file right now is background indexing, such a check is not needed.
This way we can guarantee that all files passed to `UpdateIndexStoreTaskDescription` belong to the same target, which will simplify multi-file indexing.
When I added the log structure to `build/logMessage` in #2022 I must have assumed that the entire BSP notifciation was an extension defined by SourceKit-LSP and didn’t realized that this was actually a change that made the notification non-compliant with BSP. Change it up a little bit to make it compliant again.
The term *build system* predated our wide-spread adoption of BSP for communicating between SourceKit-LSP to the build system and was never really the correct term anyway – ie. a `JSONCompilationDatabaseBuildSystem` never really sounded right. We now have a correct term for the communication layer between SourceKit-LSP: A build server. Rename most occurrences of *build system* to *build server* to reflect this. There are unfortunately a couple lingering instances of *build system* that we can’t change, most notably: `fallbackBuildSystem` in the config file, the `workspace/waitForBuildSystemUpdates` BSP extension request and the `synchronize-for-build-system-updates` experimental feature.
This is what `shutDown()` is documented to do. I also remember having this check before, it might have gotten lost during a rebase when I was working on https://github.com/swiftlang/sourcekit-lsp/pull/2081.
I noticed this while investigating https://github.com/swiftlang/sourcekit-lsp/pull/2152: In this case `buildTarget/prepare` was cancelled because the SourceKit-LSP server was shut down but indexing of a file was still started after the shutdown now that preparation had finished (because it was cancelled).
A queued task might have been cancelled after the execution ask was started but before the task was yielded to `executionTaskCreatedContinuation`. In that case the result task will simply cancel the await on the `executionTaskCreatedStream` and hence not call `valuePropagatingCancellation` on the execution task. This means that the queued task cancellation wouldn't be propagated to the execution task. To address this, check if `resultTaskCancelled` was set and, if so, explicitly cancel the execution task here.
Fixes an issue I saw in CI during PR testing.
When SourceKit-LSP is shut down, we should make sure that we don’t leave behind child processes, which will become orphans after SourceKit-LSP has terminated. What’s worse, when SourceKit-LSP has exited, these processes might not have any process to read their stdout/stderr, which can lead to them running indefinitely.
This change does not cover the termination of subprocess trees. For example, if we launch `swift build` and need to kill it because it doesn’t honor SIGINT, its child processes will still live on. Similarly, if we kill a BSP server, its child processes might live on. Fixing this is a drastically bigger endeavor, likely requiring changes to Foundation and/or TSC. I filed https://github.com/swiftlang/sourcekit-lsp/issues/2080 for it.
Rintaro Ishizaki
Alex Hoppen
Bruno Rocha
Ben Barham
Josh Arnold
Anthony Latsis
Mishal Shah
Michael Gottesman
Owen Voorhees
Matthew Bastien