Skip to content

Commit 04944fa

Browse files
committed
internal/lsp: fix deadlocks loading lots of files at once
The first deadlock involved differing mutex acquisition order in two code paths: 1. loadParseTypecheck() holds the "mcache" mutex then eventually acquires the "handleMu" file mutex. 2. (*goFile).invalidateContent() acquires the "handleMu" mutex first and then the "mcache" mutex. Fix by changing the acquisition order in invalidateContent(). The second deadlock involved the file watcher. The two code paths involved were: 1. (*goFile).GetPackages() holds the view mutex and eventually gets (*WatchMap).Watch, which acquires the watcher mutex. 2. (*session).openOverlay acquires the watcher mutex as it triggers a file's callbacks, and then the callback "(*goFile).invalidateContent" acquires the vie mutex. Fix by not holding the watcher mutex as we invoke the callbacks.
1 parent 9d59f9e commit 04944fa

File tree

2 files changed

+13
-5
lines changed

2 files changed

+13
-5
lines changed

internal/lsp/cache/view.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,15 +225,17 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) err
225225
// invalidateContent invalidates the content of a Go file,
226226
// including any position and type information that depends on it.
227227
func (f *goFile) invalidateContent(ctx context.Context) {
228-
f.handleMu.Lock()
229-
defer f.handleMu.Unlock()
230-
228+
// Mutex acquisition order here is important. It must match the order
229+
// in loadParseTypecheck to avoid deadlocks.
231230
f.view.mcache.mu.Lock()
232231
defer f.view.mcache.mu.Unlock()
233232

234233
f.view.pcache.mu.Lock()
235234
defer f.view.pcache.mu.Unlock()
236235

236+
f.handleMu.Lock()
237+
defer f.handleMu.Unlock()
238+
237239
f.invalidateAST(ctx)
238240
f.handle = nil
239241
}

internal/lsp/cache/watcher.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,15 @@ func (w *WatchMap) Watch(key interface{}, callback func()) func() {
4848
}
4949

5050
func (w *WatchMap) Notify(key interface{}) {
51+
// Make a copy of the watcher callbacks so we don't need to hold
52+
// the mutex during the callbacks (to avoid deadlocks).
5153
w.mu.Lock()
52-
defer w.mu.Unlock()
53-
for _, entry := range w.watchers[key] {
54+
entries := w.watchers[key]
55+
entriesCopy := make([]watcher, len(entries))
56+
copy(entriesCopy, entries)
57+
w.mu.Unlock()
58+
59+
for _, entry := range entriesCopy {
5460
entry.callback()
5561
}
5662
}

0 commit comments

Comments
 (0)