search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.13k stars 17.56k forks source link

x/tools/gopls: reload metadata on the most durable awaiting context #43652

Open findleyr opened 3 years ago

findleyr commented 3 years ago

We try to avoid cloning a snapshot when the previous snapshot has not initialized, by awaiting on a detached context: https://cs.opensource.google/go/x/tools/+/master:internal/lsp/cache/view.go;l=602;drc=929a8494cf60267d89035bc211b797a67b65a6b9

However, we also guard initialization with a sync.Once. As has been observed in #43554, it's possible to lose this race to awaitInitialized, such that we run initialization on the snapshot background context, which can be canceled.

As a result, we can clone a snapshot with incomplete metadata. This causes problems for any invalidation logic trying to preserve metadata, such as only invalidating metadata on go.mod saves. Additionally, this was difficult to debug: it would be nice to have a guarantee that we have a linear history of metadata in our snapshots.

After discussing, we think this is a relatively low priority since the current logic should only result in rare and transient breakages.

CC @stamblerre @heschik

gopherbot commented 3 years ago

Change https://golang.org/cl/283512 mentions this issue: gopls/internal/regtest: fix synchronization for TestUseGoplsMod