internal/memoize: record the caller of Destroy

The hypothesis for golang/go#48774 is that the generation is being destroyed by
a call to (*View).shutdown. This change adds a bit of logging to
confirm that hypothesis.

For golang/go#48774

Change-Id: I34be2e16a0dcab4cea7e9b704b56f4cf0abb0c71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/367674
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

internal/lsp/cache/view.go

@@ -505,7 +505,7 @@ func (v *View) shutdown(ctx context.Context) {
 	}
 	v.mu.Unlock()
 	v.snapshotMu.Lock()
-	go v.snapshot.generation.Destroy()
+	go v.snapshot.generation.Destroy("View.shutdown")
 	v.snapshotMu.Unlock()
 	v.importsState.destroy()
 }
@@ -694,7 +694,7 @@ func (v *View) invalidateContent(ctx context.Context, changes map[span.URI]*file
 			event.Error(ctx, "copying workspace dir", err)
 		}
 	}
-	go oldSnapshot.generation.Destroy()
+	go oldSnapshot.generation.Destroy("View.invalidateContent")
 
 	return v.snapshot, v.snapshot.generation.Acquire(ctx)
 }

internal/memoize/memoize.go

@@ -62,6 +62,8 @@ type Generation struct {
 	destroyed uint32
 	store     *Store
 	name      string
+	// destroyedBy describes the caller that togged destroyed from 0 to 1.
+	destroyedBy string
 	// wg tracks the reference count of this generation.
 	wg sync.WaitGroup
 }
@@ -69,10 +71,16 @@ type Generation struct {
 // Destroy waits for all operations referencing g to complete, then removes
 // all references to g from cache entries. Cache entries that no longer
 // reference any non-destroyed generation are removed. Destroy must be called
-// exactly once for each generation.
-func (g *Generation) Destroy() {
+// exactly once for each generation, and destroyedBy describes the caller.
+func (g *Generation) Destroy(destroyedBy string) {
 	g.wg.Wait()
-	atomic.StoreUint32(&g.destroyed, 1)
+
+	prevDestroyedBy := g.destroyedBy
+	g.destroyedBy = destroyedBy
+	if ok := atomic.CompareAndSwapUint32(&g.destroyed, 0, 1); !ok {
+		panic("Destroy on generation " + g.name + " already destroyed by " + prevDestroyedBy)
+	}
+
 	g.store.mu.Lock()
 	defer g.store.mu.Unlock()
 	for k, e := range g.store.handles {
@@ -100,7 +108,7 @@ func (g *Generation) Acquire(ctx context.Context) func() {
 		return func() {}
 	}
 	if destroyed != 0 {
-		panic("acquire on destroyed generation " + g.name)
+		panic("acquire on generation " + g.name + " destroyed by " + g.destroyedBy)
 	}
 	g.wg.Add(1)
 	return g.wg.Done
@@ -175,7 +183,7 @@ func (g *Generation) Bind(key interface{}, function Function, cleanup func(inter
 		panic("the function passed to bind must not be nil")
 	}
 	if atomic.LoadUint32(&g.destroyed) != 0 {
-		panic("operation on destroyed generation " + g.name)
+		panic("operation on generation " + g.name + " destroyed by " + g.destroyedBy)
 	}
 	g.store.mu.Lock()
 	defer g.store.mu.Unlock()
@@ -233,7 +241,7 @@ func (s *Store) DebugOnlyIterate(f func(k, v interface{})) {
 func (g *Generation) Inherit(hs ...*Handle) {
 	for _, h := range hs {
 		if atomic.LoadUint32(&g.destroyed) != 0 {
-			panic("inherit on destroyed generation " + g.name)
+			panic("inherit on generation " + g.name + " destroyed by " + g.destroyedBy)
 		}
 
 		h.mu.Lock()

internal/memoize/memoize_test.go

@@ -60,11 +60,11 @@ func TestGenerations(t *testing.T) {
 	expectGet(t, h2, g2, "res")
 
 	// With g1 destroyed, g2 should still work.
-	g1.Destroy()
+	g1.Destroy("TestGenerations")
 	expectGet(t, h2, g2, "res")
 
 	// With all generations destroyed, key should be re-evaluated.
-	g2.Destroy()
+	g2.Destroy("TestGenerations")
 	g3 := s.Generation("g3")
 	h3 := g3.Bind("key", func(context.Context, memoize.Arg) interface{} { return "new res" }, nil)
 	expectGet(t, h3, g3, "new res")
@@ -89,7 +89,7 @@ func TestCleanup(t *testing.T) {
 	g2 := s.Generation("g2")
 	g2.Inherit(h1, h2)
 
-	g1.Destroy()
+	g1.Destroy("TestCleanup")
 	expectGet(t, h1, g2, &v1)
 	expectGet(t, h2, g2, &v2)
 	for k, v := range map[string]*bool{"key1": &v1, "key2": &v2} {
@@ -97,7 +97,7 @@ func TestCleanup(t *testing.T) {
 			t.Errorf("after destroying g1, bound value %q is cleaned up", k)
 		}
 	}
-	g2.Destroy()
+	g2.Destroy("TestCleanup")
 	if got, want := v1, false; got != want {
 		t.Error("after destroying g2, v1 is cleaned up")
 	}