Add an LSP server command #3316
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
private/buf/buflsp/file_manager.go
Outdated
| } | ||
|
|
||
| // newFiles creates a new file manager. | ||
| func newFiles(lsp *lsp) *fileManager { |
|
|
||
| func (mu *mutex) storeWho(id uint64) { | ||
| for { | ||
| // This has to be a CAS loop to avoid races with a poisoning p. |
There was a problem hiding this comment.
Not really. If you are unable to swap it's confirmed to be poisoned as the lock is always held.
if !mu.who.CompareAndSwap(0, id) {
panic("buflsp/mutex.go: non-reentrant lock locked twice by the same request")
}| mu.storeWho(0) | ||
|
|
||
| mu.pool.check(id, mu, true) | ||
| mu.lock.Unlock() |
There was a problem hiding this comment.
The mu.lock.Unlock() should be deferred, as this never gets called on panic of storeWho. So go routines will still deadlock on line 150 as they wait for this Unlock. Although does this matter? Would only matter if using recovers.
There was a problem hiding this comment.
@mcy can you check over all of your concurrency logic here before we merge?
There was a problem hiding this comment.
Yeah -- I think that's the crux of this, it seems to me that panic's are used under the assumption that we are crashing the program to avoid attempting to recover from bad code-patterns (and thus, hopefully, preventing us from merging bad patterns). That being said, deferring here to make the pattern with storeWho a little less fragile is probably a good thing.
private/buf/buflsp/file_manager.go
Outdated
| // by the editor. | ||
| type fileManager struct { | ||
| lsp *lsp | ||
| table refcount.Map[protocol.URI, file] |
There was a problem hiding this comment.
@doriable can you do a pass on naming here? None of these names match our style
This PR adds a new command, buf beta lsp, which serves an LSP for Protobuf files.
Most of the code lives under
buflsp. Currently, the LSP implements: