Skip SourceKit requests once sourcekitd is known unavailable - #6762
Closed
Brett-Best wants to merge 8 commits into
Closed
Brett-Best wants to merge 8 commits into
Brett-Best wants to merge 8 commits into
Conversation
# Conflicts: # Tests/BuiltInRulesTests/FileHeaderRuleTests.swift # Tests/BuiltInRulesTests/FileNameNoSpaceRuleTests.swift # Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift # Tests/CoreTests/RegexConfigurationTests.swift # Tests/CoreTests/SwiftLintFileTests.swift # Tests/FileSystemAccessTests/BaselineTests.swift # Tests/FileSystemAccessTests/ConfigurationTests+Mock.swift # Tests/FileSystemAccessTests/ConfigurationTests.swift # Tests/FileSystemAccessTests/GlobTests.swift # Tests/FileSystemAccessTests/MultipleConfigurationsTests.swift # Tests/FileSystemAccessTests/ReporterTests.swift # Tests/FileSystemAccessTests/SourceKitCrashTests.swift # Tests/FrameworkTests/CustomRulesTests.swift # Tests/FrameworkTests/LinterCacheTests.swift # Tests/FrameworkTests/RuleConfigurationTests.swift # Tests/FrameworkTests/SwiftVersionTests.swift # Tests/GeneratedTests/GeneratedTests_02.swift # Tests/GeneratedTests/GeneratedTests_03.swift # Tests/GeneratedTests/GeneratedTests_04.swift # Tests/GeneratedTests/GeneratedTests_05.swift # Tests/GeneratedTests/GeneratedTests_06.swift # Tests/GeneratedTests/GeneratedTests_07.swift # Tests/GeneratedTests/GeneratedTests_08.swift # Tests/GeneratedTests/GeneratedTests_09.swift # Tests/GeneratedTests/GeneratedTests_10.swift # Tests/IntegrationTests/ConfigPathResolutionTests.swift # Tests/IntegrationTests/IntegrationTests.swift # Tests/TestHelpers/TestHelpers.swift
* Disable tests on macOS 14/Xcode 16.2 * Remove `--parallel` which is the default * Set reasonable test timeouts
This reverts commit 5847df9.
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
June 7, 2026 10:59
54e0f30 to
9610daa
Compare
SimplyDanny
force-pushed
the
adopt-swift-testing
branch
15 times, most recently
from
June 16, 2026 18:50
129a1f9 to
1fca438
Compare
Collaborator
|
Thanks for the idea, @Brett-Best. As it turns out, the reason for the hang is not that SourceKit failed (then we should fix the fail), but that it gets overwhelmed by the number of requests it receives. This can be reproduced even with a small number of parallel threads. Not sure whether this is an issue of SourceKit or due to the way we use it. In any case, this seems only to be an issue in parallel testing, so running them sequentially for now should be fine until there is a proper solution. |
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