Skip SourceKit requests once sourcekitd is known unavailable#6762
Open
Brett-Best wants to merge 1 commit into
Open
Skip SourceKit requests once sourcekitd is known unavailable#6762Brett-Best wants to merge 1 commit into
Brett-Best wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a process-wide “SourceKit unavailable” latch to avoid Swift Testing hangs when sourcekitd is down, and updates a rule path and tests to exercise the new short-circuit behavior.
Changes:
- Introduces
SourceKitStatus+SourceKitUnavailableErrorto short-circuit SourceKit requests oncesourcekitdis deemed unavailable. - Wraps
Request.sendIfNotDisabled()to record success/failure and skip future requests when latched unavailable. - Adds a regression test and updates
UnusedDeclarationRuleindexing to usesendIfNotDisabled().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Tests/FileSystemAccessTests/SourceKitCrashTests.swift | Adds a regression test ensuring SourceKit requests fail fast when forced unavailable. |
| Source/SwiftLintCore/Extensions/Request+SwiftLint.swift | Adds SourceKitStatus latch + short-circuit error to prevent executor starvation/hangs. |
| Source/SwiftLintBuiltInRules/Rules/Lint/UnusedDeclarationRule.swift | Routes indexing request through sendIfNotDisabled() so it participates in the new behavior. |
Comment on lines
+116
to
+126
| guard !SourceKitStatus.isUnavailable else { | ||
| throw SourceKitUnavailableError() | ||
| } | ||
| do { | ||
| let response = try send() | ||
| SourceKitStatus.markSucceeded() | ||
| return response | ||
| } catch { | ||
| SourceKitStatus.markFailed() | ||
| throw error | ||
| } |
Comment on lines
+52
to
+58
| /// Records a failed sourcekitd request. This only latches sourcekitd as globally unavailable if | ||
| /// no request has ever succeeded — i.e. sourcekitd never came up, rather than a one-off crash. | ||
| public static func markFailed() { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| everFailed = true | ||
| } |
Comment on lines
+23
to
+24
| nonisolated(unsafe) private static var everSucceeded = false | ||
| nonisolated(unsafe) private static var everFailed = false |
Here's an example of your CHANGELOG entry: * Skip SourceKit requests once sourcekitd is known unavailable.
[Brett-Best](https://github.com/Brett-Best)
[#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)note: There are two invisible spaces after the entry's text. Generated by 🚫 Danger |
…ging Under Swift Testing the suite runs as many concurrent tasks on a small cooperative executor. Each linted file issues a synchronous, blocking sourcekitd request; if the daemon wedges — as it does on the macOS CI runners — every concurrent caller blocks on it and the whole run hangs until the job times out, with zero tests completing. Run each request through `Request.sendIfNotDisabled()` with a 30s timeout: a wedged request now frees its cooperative-executor thread instead of blocking forever, and the first time a request times out `SourceKitStatus` latches so the remaining requests skip sourcekitd rather than each paying the timeout. A healthy daemon answers immediately, so the latch never trips and behaviour is unchanged. `UnusedDeclarationRule`'s index request previously called `send()` directly, bypassing the timeout; route it through `sendIfNotDisabled()` like the others. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brett-Best
force-pushed
the
fix/sourcekit-unavailable-hang-latch
branch
from
54e0f30 to
9610daa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targets the
adopt-swift-testingbranch (#6048).Problem
On
adopt-swift-testingthe macOS CI test jobs hang and are cancelled at the 10‑minute timeout (Linux/Windows pass). In the cancelled run the whole suite printsstartedin one burst, thensourcekitd has failedat ~T+2s, then zero tests complete for 8+ minutes.Root cause: Swift Testing runs every test as a concurrent task on a bounded cooperative executor (XCTest used its own per-test threads). Each linted file issues a synchronous, blocking sourcekitd request. When sourcekitd is unavailable, each request fails fast (~2s) but there is no global short‑circuit, so all ~1000 files independently re‑issue their own doomed request. Across the few cooperative threads that is >10 minutes of nothing-but-failing-requests, which starves the executor so no other test can make progress.
Fix
Add a process‑global
SourceKitStatuslatch, enforced centrally inRequest.sendIfNotDisabled()(so it coverseditorOpen,index, andcursorInfouniformly):isUnavailablelatchestrueand subsequent requests throwSourceKitUnavailableErrorimmediately instead of blocking. The cascade collapses to a single failed request.SwiftLintFile.sourcekitdFailed/assertHandler); such a crash happens after sourcekitd has already answered other files, soeverSucceededis already set and the global latch never engages. On a healthy machine the first request succeeds, so the latch is inert — no behaviour change.Also routes
UnusedDeclarationRule'sindexrequest throughsendIfNotDisabled()(it previously calledsend()directly, bypassing the latch).Testing
SourceKitCrashTests.sourceKitUnavailableShortCircuitsWithoutIssuingRequest): forces the unavailable state for one task tree (isolated from parallel tests via a@TaskLocal) and asserts the central choke point throws without issuing a request. It fails before the fix and passes after.1067 tests / 370 suites),swiftlint lint --strictclean.Notes / caveats
SwiftVersionstartup probe) fails transiently on an otherwise‑healthy machine, the latch trips for the run. In practice this is rare, and it is actually beneficial on CI (trips early, before file processing).🤖 Generated with Claude Code