getOrAddWeakHandle is very careful about keeping its input alive across
the operation, but not very careful about keeping the heap-allocated
handle it creates alive. In fact, there's a window in this function
where it is *only* visible via the special. Specifically, the window of
time between when the handle is stored in the special and when the
special actually becomes visible to the GC.
(If we fail to add the special because it already exists, that case is
fine. We don't even use the same handle value, but the one we obtain
from the attached GC-visible special, *and* we return that value, so it
remains live.)
Fixes #70455.
Change-Id: Iadaff0cfb93bcaf61ba2b05be7fa0519c481de82
Reviewed-on: https://go-review.googlesource.com/c/go/+/630315
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
// Keep p alive for the duration of the function to ensure
// that it cannot die while we're trying to do this.
+ //
+ // Same for handle, which is only stored in the special.
+ // There's a window where it might die if we don't keep it
+ // alive explicitly. Returning it here is probably good enough,
+ // but let's be defensive and explicit. See #70455.
KeepAlive(p)
- return s.handle
+ KeepAlive(handle)
+ return handle
}
// There was an existing handle. Free the special
// Keep p alive for the duration of the function to ensure
// that it cannot die while we're trying to do this.
+ //
+ // Same for handle, just to be defensive.
KeepAlive(p)
+ KeepAlive(handle)
return handle
}