Some sourcekitd documents opened in SwiftLanguageServer are never closed

[ianhanken]

In SwiftLanguageServer, there are a few cases where sourcekitd files are opened using source.request.editor.open, but these file names are prepended by an extra string to indicate that they are not directly related to a file URI. Below is a snippet as well as the lines where this occurs:

let skreq = SKDRequestDictionary(sourcekitd: self.sourcekitd)
skreq[keys.request] = self.requests.editor_open
skreq[keys.name] = "FoldingRanges:" + snapshot.document.uri.pseudoPath
skreq[keys.sourcetext] = snapshot.text
skreq[keys.syntactic_only] = 1

https://github.com/apple/sourcekit-lsp/blob/main/Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift#L664
https://github.com/apple/sourcekit-lsp/blob/main/Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift#L761
https://github.com/apple/sourcekit-lsp/blob/main/Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift#L967

These documents don't seem to have a source.request.editor.close, so they are likely never closed and remain open for the duration of the SourceKit-LSP session.

[DavidGoldman]

Also, is there is a reason to use a separate document for these operations even though we've already opened one for the document (which isn't syntactic_only)?

[ahoppen]

Also, is there is a reason to use a separate document for these operations even though we've already opened one for the document (which isn't syntactic_only)?

IIRC the reason is that if sourcekitd crashes, it will disable semantic functionality for ~10 seconds. In these cases we still want to provide syntactic functionality, which we can’t do if we request semantic functionality for the document, so we just open a second syntactic-only copy for it.

[ahoppen]

rdar://93154201

[DavidGoldman]

IIRC the reason is that if sourcekitd crashes, it will disable semantic functionality for ~10 seconds. In these cases we still want to provide syntactic functionality, which we can’t do if we request semantic functionality for the document, so we just open a second syntactic-only copy for it.

Hmm, if crashing is relatively rare, I wonder if it would make more sense in the normal case to reuse the already open document, and then maybe special case the behavior when semantic functionality is disabled (or maybe have sourcekitd handle this logic for us)?

[ahoppen]

I think that would be a reasonable change.

[benlangmuir]

Also, is there is a reason to use a separate document for these operations even though we've already opened one for the document (which isn't syntactic_only)?

This requires keeping the state of all the information you want across edits, including handling incremental updates to token locations. I think now we may have the infrastructure to do that, due to the work on syntax highlighting, but at the time this code was added we did not and it was much simpler to just re-parse. Probably worth re-evaluating now.

In these cases we still want to provide syntactic functionality, which we can’t do if we request semantic functionality for the document, so we just open a second syntactic-only copy for it.

I thought we still opened the normal documents and just didn't trigger any typechecking until after the semantic delay was up. The syntactic parts should just work still.