x/tools/gopls: deadlock opening many files at once #32910
Labels
Milestone
Comments
|
@ianthehat while testing out a fix for above deadlock I hit a different deadlock involving the watcher. Basically one codepath tries adding a file to the watcher while holding the view's mutex, and another codepath is invoking a watcher callback (which holds the watcher mutex) and the callback acquires the view mutex. I have annotated the relevant stack traces below with @@mutex. Do you think we should make the watcher not hold the mutex while invoking the callbacks (i.e. copy the slice of callbacks before invoking), or perhaps invoke the callbacks in a goroutine (or something else)? |
|
Change https://golang.org/cl/184880 mentions this issue: |
movie-travel-code
pushed a commit
to movie-travel-code/tools
that referenced
this issue
Jul 11, 2019
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 calls (*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 view mutex. Fix by not holding the watcher mutex as we invoke the callbacks. Fixes golang/go#32910 Change-Id: I9d060e0d80fd86a317a1d6c7aaa736a8ce10bd07 GitHub-Last-Rev: 04944fa GitHub-Pull-Request: golang#129 Reviewed-on: https://go-review.googlesource.com/c/tools/+/184880 Reviewed-by: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
While testing a different issue I ran into a new deadlock. I think it boils down to a different lock acquisition order in these two code paths:
Path 1:
loadParseTypecheckacquires mcache mutexlsp/cache.(*fileBase).Handlewhich acquiresf.handleMuPath 2:
lsp/cache.(*goFile).invalidateContentacquiresf.handleMuand then tries acquiringmcachemutex.Here is the full goroutine trace:
/cc @stamblerre
The text was updated successfully, but these errors were encountered: