x/tools/gopls: deadlock opening many files at once #32910
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:
Here is the full goroutine trace:
@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)?
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 <firstname.lastname@example.org> Run-TryBot: Rebecca Stambler <email@example.com> TryBot-Result: Gobot Gobot <firstname.lastname@example.org>