cmd/go: keep objects alive while stopping cleanups

There are two places in cmd/go where cleanups are stopped before they
fire, and where the objects the cleanups are attached to may be dead
while we call Stop. This is essentially a race between Stop and the
cleanup being called. This can be fine, but these cleanups are used as
a way to check some invariants, so just panic if they're executed. As a
result, if they fire erroneously, they'll take down the whole process,
even if no invariant was actually violated.

The runtime.Cleanup.Stop documentation explains that users of Stop need
to hold the object alive across the call to Stop if they want to be sure
that Stop succeeds, so do that here by adding an explicit
runtime.KeepAlive call.

Kudos to Michael Pratt for finding the issue.

Fixes #74780.

Change-Id: I22e6f4642ac68f727ca3781f5d39a85015047925
Reviewed-on: https://go-review.googlesource.com/c/go/+/719961
Auto-Submit: Michael Knyszek <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>

Author: mknyszek

src/cmd/go/internal/base/limit.go

@@ -63,6 +63,12 @@ func AcquireNet() (release func(), err error) {
 			<-netLimitSem
 		}
 		cleanup.Stop()
+
+		// checker may be dead at the moment after we last access
+		// it in this function, so the cleanup can fire before Stop
+		// completes. Keep checker alive while we call Stop. See
+		// the documentation for runtime.Cleanup.Stop.
+		runtime.KeepAlive(checker)
 	}, nil
 }
 

src/cmd/go/internal/lockedfile/lockedfile.go

@@ -94,6 +94,11 @@ func (f *File) Close() error {
 
 	err := closeFile(f.osFile.File)
 	f.cleanup.Stop()
+	// f may be dead at the moment after we access f.cleanup,
+	// so the cleanup can fire before Stop completes. Keep f
+	// alive while we call Stop. See the documentation for
+	// runtime.Cleanup.Stop.
+	runtime.KeepAlive(f)
 	return err
 }