Skip to content

Commit 7ec52e3

Browse files
committed
Delay restarting cland in case of a crash loop
1 parent 31e9c15 commit 7ec52e3

File tree

2 files changed

+93
-12
lines changed

2 files changed

+93
-12
lines changed

Sources/SourceKit/clangd/ClangLanguageServer.swift

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ final class ClangLanguageServerShim: ToolchainLanguageServer {
4444
}
4545
}
4646
}
47+
48+
/// The date at which `clangd` was last restarted. Used to delay restarting in case of a crash loop.
49+
private var lastClangdRestart: Date?
50+
51+
/// Whether or not a restart of `clangd` has been scheduled. Used to make sure we are not restarting `clangd` twice.
52+
private var clangRestartScheduled = false
4753

4854
private var stateChangeHandlers: [(_ oldState: LanguageServerState, _ newState: LanguageServerState) -> Void] = []
4955

@@ -118,23 +124,39 @@ final class ClangLanguageServerShim: ToolchainLanguageServer {
118124

119125
private func restartClangd() {
120126
precondition(self.state == .connectionInterrupted)
127+
128+
precondition(clangRestartScheduled == false)
129+
clangRestartScheduled = true
121130

122131
guard let initializeRequest = initializeRequest else {
123132
log("clangd crashed before it was sent an InitializeRequest.", level: .error)
124133
return
125134
}
126-
127-
do {
128-
try self.initialize()
129-
// FIXME: We assume that clangd will return the same capabilites after restarting.
130-
// Theoretically they could have changed and we would need to inform SourceKitServer about them.
131-
// But since SourceKitServer more or less ignores them right now anyway, this should be fine for now.
132-
_ = try self.initializeSync(initializeRequest)
133-
self.clientInitialized(InitializedNotification())
134-
self.reopenDocuments(self)
135-
self.state = .connected
136-
} catch {
137-
log("Failed to restart clangd after a crash.", level: .error)
135+
136+
let restartDelay: Int
137+
if let lastClangdRestart = lastClangdRestart, Date().timeIntervalSince(lastClangdRestart) < 30 {
138+
// We have crashed in the last 30 seconds. Delay any further restart by 10 seconds
139+
log("clangd crashed in the last 30 seconds. Delaying another restart by 10 seconds.", level: .info)
140+
restartDelay = 10
141+
} else {
142+
restartDelay = 0
143+
}
144+
lastClangdRestart = Date()
145+
146+
DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + .seconds(restartDelay)) {
147+
self.clangRestartScheduled = false
148+
do {
149+
try self.initialize()
150+
// FIXME: We assume that clangd will return the same capabilites after restarting.
151+
// Theoretically they could have changed and we would need to inform SourceKitServer about them.
152+
// But since SourceKitServer more or less ignores them right now anyway, this should be fine for now.
153+
_ = try self.initializeSync(initializeRequest)
154+
self.clientInitialized(InitializedNotification())
155+
self.reopenDocuments(self)
156+
self.state = .connected
157+
} catch {
158+
log("Failed to restart clangd after a crash.", level: .error)
159+
}
138160
}
139161
}
140162

Tests/SourceKitTests/CrashRecoveryTests.swift

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,5 +206,64 @@ final class CrashRecoveryTests: XCTestCase {
206206
XCTAssertEqual(postCrashHighlightResponse, expectedHighlightResponse)
207207
}())
208208
}
209+
210+
func testPreventClangdCrashLoop() {
211+
guard longTestsEnabled else {
212+
return
213+
}
214+
215+
let ws = try! staticSourceKitTibsWorkspace(name: "ClangCrashRecovery")!
216+
let loc = ws.testLoc("loc")
217+
218+
try! ws.openDocument(loc.url, language: .cpp)
219+
220+
// Send a non-sential request to wait for clangd to start up
221+
222+
let hoverRequest = HoverRequest(textDocument: loc.docIdentifier, position: Position(line: 1, utf16index: 6))
223+
_ = try! ws.sk.sendSync(hoverRequest)
224+
225+
// Start crashing clangd
226+
227+
let clangdServer = ws.testServer.server!.languageService(for: loc.docUri, .cpp, in: ws.testServer.server!.workspace!)!
228+
229+
let clangdCrashed = self.expectation(description: "clangd crashed")
230+
clangdCrashed.assertForOverFulfill = false
231+
let clangdRestartedFirstTime = self.expectation(description: "clangd restarted for the first time")
232+
let clangdRestartedSecondTime = self.expectation(description: "clangd restarted for the second time")
233+
clangdRestartedSecondTime.assertForOverFulfill = false
234+
var clangdHasRestartedFirstTime = false
235+
236+
clangdServer.addStateChangeHandler { (oldState, newState) in
237+
switch newState {
238+
case .connectionInterrupted:
239+
clangdCrashed.fulfill()
240+
case .connected:
241+
if !clangdHasRestartedFirstTime {
242+
clangdRestartedFirstTime.fulfill()
243+
clangdHasRestartedFirstTime = true
244+
} else {
245+
clangdRestartedSecondTime.fulfill()
246+
}
247+
default:
248+
break
249+
}
250+
}
251+
252+
// Add a pragma to crash clang
253+
254+
let addCrashPragma = TextDocumentContentChangeEvent(range: loc.position..<loc.position, rangeLength: 0, text: "#pragma clang __debug crash\n")
255+
ws.sk.send(DidChangeTextDocumentNotification(textDocument: VersionedTextDocumentIdentifier(loc.docUri, version: 3), contentChanges: [addCrashPragma]))
256+
257+
self.wait(for: [clangdCrashed], timeout: 5)
258+
259+
// Clangd has crashed for the first time. Leave the pragma there to crash clangd once again.
260+
261+
self.wait(for: [clangdRestartedFirstTime], timeout: 30)
262+
// Clangd has restarted. Note the date so we can check that the second restart doesn't happen too quickly.
263+
let firstRestartDate = Date()
264+
265+
self.wait(for: [clangdRestartedSecondTime], timeout: 30)
266+
XCTAssert(Date().timeIntervalSince(firstRestartDate) > 5, "Clangd restarted too quickly after crashing twice in a row. We are not preventing crash loops.")
267+
}
209268
}
210269

0 commit comments

Comments
 (0)