Replace pkg/refs with pkg/refsvfs2.

All VFS1 only bits have been deleted.

Updates #1624

PiperOrigin-RevId: 492273183

pkg/bufferv2/BUILD

@@ -8,7 +8,7 @@ go_template_instance(
     out = "chunk_refs.go",
     package = "bufferv2",
     prefix = "chunk",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "chunk",
     },
@@ -45,7 +45,7 @@ go_library(
         "//pkg/ilist",
         "//pkg/log",
         "//pkg/pool",
-        "//pkg/refsvfs2",
+        "//pkg/refs",
         "//pkg/sync",
         "//pkg/tcpip/checksum",
     ],

pkg/lisafs/BUILD

@@ -11,7 +11,7 @@ go_template_instance(
     out = "control_fd_refs.go",
     package = "lisafs",
     prefix = "controlFD",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "ControlFD",
     },
@@ -22,7 +22,7 @@ go_template_instance(
     out = "open_fd_refs.go",
     package = "lisafs",
     prefix = "openFD",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "OpenFD",
     },
@@ -33,7 +33,7 @@ go_template_instance(
     out = "bound_socket_fd_refs.go",
     package = "lisafs",
     prefix = "boundSocketFD",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "BoundSocketFD",
     },
@@ -44,7 +44,7 @@ go_template_instance(
     out = "node_fd_refs.go",
     package = "lisafs",
     prefix = "node",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "Node",
     },
@@ -110,7 +110,7 @@ go_library(
         "//pkg/log",
         "//pkg/marshal/primitive",
         "//pkg/p9",
-        "//pkg/refsvfs2",
+        "//pkg/refs",
         "//pkg/sync",
         "//pkg/unet",
         "@org_golang_x_sys//unix:go_default_library",

pkg/lisafs/fd.go

@@ -18,7 +18,7 @@ import (
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
-	"gvisor.dev/gvisor/pkg/refsvfs2"
+	"gvisor.dev/gvisor/pkg/refs"
 	"gvisor.dev/gvisor/pkg/sync"
 )
 
@@ -38,7 +38,7 @@ func (f FDID) Ok() bool {
 
 // genericFD can represent any type of FD.
 type genericFD interface {
-	refsvfs2.RefCounter
+	refs.RefCounter
 }
 
 // A ControlFD is the gateway to the backing filesystem tree node. It is an
@@ -84,9 +84,9 @@ type ControlFD struct {
 
 var _ genericFD = (*ControlFD)(nil)
 
-// DecRef implements refsvfs2.RefCounter.DecRef. Note that the context
+// DecRef implements refs.RefCounter.DecRef. Note that the context
 // parameter should never be used. It exists solely to comply with the
-// refsvfs2.RefCounter interface.
+// refs.RefCounter interface.
 func (fd *ControlFD) DecRef(context.Context) {
 	fd.controlFDRefs.DecRef(func() {
 		fd.conn.server.renameMu.RLock()
@@ -254,9 +254,9 @@ func (fd *OpenFD) ControlFD() ControlFDImpl {
 	return fd.controlFD.impl
 }
 
-// DecRef implements refsvfs2.RefCounter.DecRef. Note that the context
+// DecRef implements refs.RefCounter.DecRef. Note that the context
 // parameter should never be used. It exists solely to comply with the
-// refsvfs2.RefCounter interface.
+// refs.RefCounter interface.
 func (fd *OpenFD) DecRef(context.Context) {
 	fd.openFDRefs.DecRef(func() {
 		fd.controlFD.openFDsMu.Lock()
@@ -311,9 +311,9 @@ func (fd *BoundSocketFD) ControlFD() ControlFDImpl {
 	return fd.controlFD.impl
 }
 
-// DecRef implements refsvfs2.RefCounter.DecRef. Note that the context
+// DecRef implements refs.RefCounter.DecRef. Note that the context
 // parameter should never be used. It exists solely to comply with the
-// refsvfs2.RefCounter interface.
+// refs.RefCounter interface.
 func (fd *BoundSocketFD) DecRef(context.Context) {
 	fd.boundSocketFDRefs.DecRef(func() {
 		fd.controlFD.DecRef(nil) // Drop the ref on the control FD.

pkg/lisafs/node.go

@@ -101,9 +101,9 @@ type Node struct {
 	dynamicChildren map[string]*Node
 }
 
-// DecRef implements refsvfs2.RefCounter.DecRef. Note that the context
+// DecRef implements refs.RefCounter.DecRef. Note that the context
 // parameter should never be used. It exists solely to comply with the
-// refsvfs2.RefCounter interface.
+// refs.RefCounter interface.
 //
 // Precondition: server's rename mutex must be at least read locked.
 func (n *Node) DecRef(context.Context) {

pkg/lisafs/testsuite/BUILD

@@ -14,7 +14,6 @@ go_library(
         "//pkg/context",
         "//pkg/lisafs",
         "//pkg/refs",
-        "//pkg/refsvfs2",
         "//pkg/unet",
         "@com_github_syndtr_gocapability//capability:go_default_library",
         "@org_golang_x_sys//unix:go_default_library",

pkg/lisafs/testsuite/testsuite.go

@@ -31,7 +31,6 @@ import (
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/lisafs"
 	"gvisor.dev/gvisor/pkg/refs"
-	"gvisor.dev/gvisor/pkg/refsvfs2"
 	"gvisor.dev/gvisor/pkg/unet"
 )
 
@@ -120,7 +119,7 @@ func RunTest(t *testing.T, tester Tester, testName string, testFn TestFunc, moun
 	// Release server resources and check for leaks. Note that leak check must
 	// happen before c.Close() because server cleans up resources on shutdown.
 	server.Destroy()
-	refsvfs2.DoRepeatedLeakCheck()
+	refs.DoRepeatedLeakCheck()
 
 	c.Close() // This should trigger client and server shutdown.
 	server.Wait()

pkg/refs/BUILD

@@ -1,26 +1,31 @@
-load("//tools:defs.bzl", "go_library", "go_test")
-load("//tools/go_generics:defs.bzl", "go_template_instance")
+load("//tools:defs.bzl", "go_library")
+load("//tools/go_generics:defs.bzl", "go_template")
 
 package(licenses = ["notice"])
 
-go_template_instance(
-    name = "weak_ref_list",
-    out = "weak_ref_list.go",
-    package = "refs",
-    prefix = "weakRef",
-    template = "//pkg/ilist:generic_list",
-    types = {
-        "Element": "*WeakRef",
-        "Linker": "*WeakRef",
-    },
+go_template(
+    name = "refs_template",
+    srcs = [
+        "refs_template.go",
+    ],
+    opt_consts = [
+        "enableLogging",
+    ],
+    types = [
+        "T",
+    ],
+    visibility = ["//:sandbox"],
+    deps = [
+        "//pkg/log",
+        "//pkg/refs",
+    ],
 )
 
 go_library(
     name = "refs",
     srcs = [
         "refcounter.go",
-        "refcounter_state.go",
-        "weak_ref_list.go",
+        "refs_map.go",
     ],
     visibility = ["//:sandbox"],
     deps = [
@@ -30,14 +35,3 @@ go_library(
         "//pkg/sync",
     ],
 )
-
-go_test(
-    name = "refs_test",
-    size = "small",
-    srcs = ["refcounter_test.go"],
-    library = ":refs",
-    deps = [
-        "//pkg/context",
-        "//pkg/sync",
-    ],
-)

pkg/refs/refcounter.go

@@ -12,234 +12,52 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package refs defines an interface for reference counted objects. It
-// also provides a drop-in implementation called AtomicRefCount.
+// Package refs defines an interface for reference counted objects.
 package refs
 
 import (
 	"bytes"
 	"fmt"
-	"reflect"
 	"runtime"
-	"sync/atomic"
 
 	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/context"
-	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/sync"
 )
 
 // RefCounter is the interface to be implemented by objects that are reference
 // counted.
-//
-// TODO(gvisor.dev/issue/1624): Get rid of most of this package and replace it
-// with refsvfs2.
 type RefCounter interface {
 	// IncRef increments the reference counter on the object.
 	IncRef()
 
-	// DecRef decrements the reference counter on the object.
-	//
-	// Note that AtomicRefCounter.DecRef() does not support destructors.
-	// If a type has a destructor, it must implement its own DecRef()
-	// method and call AtomicRefCounter.DecRefWithDestructor(destructor).
+	// DecRef decrements the object's reference count. Users of refs_template.Refs
+	// may specify a destructor to be called once the reference count reaches zero.
 	DecRef(ctx context.Context)
+}
 
-	// TryIncRef attempts to increase the reference counter on the object,
-	// but may fail if all references have already been dropped. This
-	// should be used only in special circumstances, such as WeakRefs.
+// TryRefCounter is like RefCounter but allow the ref increment to be tried.
+type TryRefCounter interface {
+	RefCounter
+
+	// TryIncRef attempts to increment the reference count, but may fail if all
+	// references have already been dropped, in which case it returns false. If
+	// true is returned, then a valid reference is now held on the object.
 	TryIncRef() bool
-
-	// addWeakRef adds the given weak reference. Note that you should have a
-	// reference to the object when calling this method.
-	addWeakRef(*WeakRef)
-
-	// dropWeakRef drops the given weak reference. Note that you should have
-	// a reference to the object when calling this method.
-	dropWeakRef(*WeakRef)
-}
-
-// A WeakRefUser is notified when the last non-weak reference is dropped.
-type WeakRefUser interface {
-	// WeakRefGone is called when the last non-weak reference is dropped.
-	WeakRefGone(ctx context.Context)
-}
-
-// WeakRef is a weak reference.
-//
-// +stateify savable
-type WeakRef struct {
-	weakRefEntry `state:"nosave"`
-
-	// obj is an atomic value that points to the refCounter.
-	obj atomic.Value `state:".(savedReference)"`
-
-	// user is notified when the weak ref is zapped by the object getting
-	// destroyed.
-	user WeakRefUser
-}
-
-// weakRefPool is a pool of weak references to avoid allocations on the hot path.
-var weakRefPool = sync.Pool{
-	New: func() any {
-		return &WeakRef{}
-	},
-}
-
-// NewWeakRef acquires a weak reference for the given object.
-//
-// An optional user will be notified when the last non-weak reference is
-// dropped.
-//
-// Note that you must hold a reference to the object prior to getting a weak
-// reference. (But you may drop the non-weak reference after that.)
-func NewWeakRef(rc RefCounter, u WeakRefUser) *WeakRef {
-	w := weakRefPool.Get().(*WeakRef)
-	w.init(rc, u)
-	return w
-}
-
-// get attempts to get a normal reference to the underlying object, and returns
-// the object. If this weak reference has already been zapped (the object has
-// been destroyed) then false is returned. If the object still exists, then
-// true is returned.
-func (w *WeakRef) get() (RefCounter, bool) {
-	rc := w.obj.Load().(RefCounter)
-	if v := reflect.ValueOf(rc); v == reflect.Zero(v.Type()) {
-		// This pointer has already been zapped by zap() below. We do
-		// this to ensure that the GC can collect the underlying
-		// RefCounter objects and they don't hog resources.
-		return nil, false
-	}
-	if !rc.TryIncRef() {
-		return nil, true
-	}
-	return rc, true
-}
-
-// Get attempts to get a normal reference to the underlying object, and returns
-// the object. If this fails (the object no longer exists), then nil will be
-// returned instead.
-func (w *WeakRef) Get() RefCounter {
-	rc, _ := w.get()
-	return rc
-}
-
-// Drop drops this weak reference. You should always call drop when you are
-// finished with the weak reference. You may not use this object after calling
-// drop.
-func (w *WeakRef) Drop(ctx context.Context) {
-	rc, ok := w.get()
-	if !ok {
-		// We've been zapped already. When the refcounter has called
-		// zap, we're guaranteed it's not holding references.
-		weakRefPool.Put(w)
-		return
-	}
-	if rc == nil {
-		// The object is in the process of being destroyed. We can't
-		// remove this from the object's list, nor can we return this
-		// object to the pool. It'll just be garbage collected. This is
-		// a rare edge case, so it's not a big deal.
-		return
-	}
-
-	// At this point, we have a reference on the object. So destruction
-	// of the object (and zapping this weak reference) can't race here.
-	rc.dropWeakRef(w)
-
-	// And now aren't on the object's list of weak references. So it won't
-	// zap us if this causes the reference count to drop to zero.
-	rc.DecRef(ctx)
-
-	// Return to the pool.
-	weakRefPool.Put(w)
-}
-
-// init initializes this weak reference.
-func (w *WeakRef) init(rc RefCounter, u WeakRefUser) {
-	// Reset the contents of the weak reference.
-	// This is important because we are reseting the atomic value type.
-	// Otherwise, we could panic here if obj is different than what it was
-	// the last time this was used.
-	*w = WeakRef{}
-	w.user = u
-	w.obj.Store(rc)
-
-	// In the load path, we may already have a nil value. So we need to
-	// check whether or not that is the case before calling addWeakRef.
-	if v := reflect.ValueOf(rc); v != reflect.Zero(v.Type()) {
-		rc.addWeakRef(w)
-	}
-}
-
-// zap zaps this weak reference.
-func (w *WeakRef) zap() {
-	// We need to be careful about types here.
-	// So reflect is involved. But it's not that bad.
-	rc := w.obj.Load()
-	typ := reflect.TypeOf(rc)
-	w.obj.Store(reflect.Zero(typ).Interface())
-}
-
-// AtomicRefCount keeps a reference count using atomic operations and calls the
-// destructor when the count reaches zero.
-//
-// Do not use AtomicRefCount for new ref-counted objects! It is deprecated in
-// favor of the refsvfs2 package.
-//
-// N.B. To allow the zero-object to be initialized, the count is offset by
-// 1, that is, when refCount is n, there are really n+1 references.
-//
-// +stateify savable
-type AtomicRefCount struct {
-	// refCount is composed of two fields:
-	//
-	//	[32-bit speculative references]:[32-bit real references]
-	//
-	// Speculative references are used for TryIncRef, to avoid a
-	// CompareAndSwap loop. See IncRef, DecRef and TryIncRef for details of
-	// how these fields are used.
-	refCount atomicbitops.Int64
-
-	// name is the name of the type which owns this ref count.
-	//
-	// name is immutable after EnableLeakCheck is called.
-	name string
-
-	// stack optionally records the caller of EnableLeakCheck.
-	//
-	// stack is immutable after EnableLeakCheck is called.
-	stack []uintptr
-
-	// mu protects the list below.
-	mu sync.Mutex `state:"nosave"`
-
-	// weakRefs is our collection of weak references.
-	weakRefs weakRefList `state:"nosave"`
 }
 
 // LeakMode configures the leak checker.
 type LeakMode uint32
 
-// TODO(gvisor.dev/issue/1624): Simplify down to two modes (on/off) once vfs1
-// ref counting is gone.
 const (
-	// UninitializedLeakChecking indicates that the leak checker has not yet been initialized.
-	UninitializedLeakChecking LeakMode = iota
-
 	// NoLeakChecking indicates that no effort should be made to check for
 	// leaks.
-	NoLeakChecking
+	NoLeakChecking LeakMode = iota
 
 	// LeaksLogWarning indicates that a warning should be logged when leaks
 	// are found.
 	LeaksLogWarning
 
-	// LeaksLogTraces indicates that a trace collected during allocation
-	// should be logged when leaks are found.
-	LeaksLogTraces
-
 	// LeaksPanic indidcates that a panic should be issued when leaks are found.
 	LeaksPanic
 )
@@ -251,8 +69,6 @@ func (l *LeakMode) Set(v string) error {
 		*l = NoLeakChecking
 	case "log-names":
 		*l = LeaksLogWarning
-	case "log-traces":
-		*l = LeaksLogTraces
 	case "panic":
 		*l = LeaksPanic
 	default:
@@ -269,14 +85,10 @@ func (l *LeakMode) Get() any {
 // String implements flag.Value.
 func (l LeakMode) String() string {
 	switch l {
-	case UninitializedLeakChecking:
-		return "uninitialized"
 	case NoLeakChecking:
 		return "disabled"
 	case LeaksLogWarning:
 		return "log-names"
-	case LeaksLogTraces:
-		return "log-traces"
 	case LeaksPanic:
 		return "panic"
 	default:
@@ -373,157 +185,6 @@ func FormatStack(pcs []uintptr) string {
 	return trace.String()
 }
 
-func (r *AtomicRefCount) finalize() {
-	var note string
-	switch LeakMode(leakMode.Load()) {
-	case NoLeakChecking:
-		return
-	case UninitializedLeakChecking:
-		note = "(Leak checker uninitialized): "
-	}
-	if n := r.ReadRefs(); n != 0 {
-		msg := fmt.Sprintf("%sAtomicRefCount %p owned by %q garbage collected with ref count of %d (want 0)", note, r, r.name, n)
-		if len(r.stack) != 0 {
-			msg += ":\nCaller:\n" + FormatStack(r.stack)
-		} else {
-			msg += " (enable trace logging to debug)"
-		}
-		log.Warningf(msg)
-	}
-}
-
-// EnableLeakCheck checks for reference leaks when the AtomicRefCount gets
-// garbage collected.
-//
-// This function adds a finalizer to the AtomicRefCount, so the AtomicRefCount
-// must be at the beginning of its parent.
-//
-// name is a friendly name that will be listed as the owner of the
-// AtomicRefCount in logs. It should be the name of the parent type, including
-// package.
-func (r *AtomicRefCount) EnableLeakCheck(name string) {
-	if name == "" {
-		panic("invalid name")
-	}
-	switch LeakMode(leakMode.Load()) {
-	case NoLeakChecking:
-		return
-	case LeaksLogTraces:
-		r.stack = RecordStack()
-	}
-	r.name = name
-	runtime.SetFinalizer(r, (*AtomicRefCount).finalize)
-}
-
-// ReadRefs returns the current number of references. The returned count is
-// inherently racy and is unsafe to use without external synchronization.
-func (r *AtomicRefCount) ReadRefs() int64 {
-	// Account for the internal -1 offset on refcounts.
-	return r.refCount.Load() + 1
-}
-
-// IncRef increments this object's reference count. While the count is kept
-// greater than zero, the destructor doesn't get called.
-//
-// The sanity check here is limited to real references, since if they have
-// dropped beneath zero then the object should have been destroyed.
-//
-//go:nosplit
-func (r *AtomicRefCount) IncRef() {
-	if v := r.refCount.Add(1); v <= 0 {
-		panic("Incrementing non-positive ref count")
-	}
-}
-
-// TryIncRef attempts to increment the reference count, *unless the count has
-// already reached zero*. If false is returned, then the object has already
-// been destroyed, and the weak reference is no longer valid. If true if
-// returned then a valid reference is now held on the object.
-//
-// To do this safely without a loop, a speculative reference is first acquired
-// on the object. This allows multiple concurrent TryIncRef calls to
-// distinguish other TryIncRef calls from genuine references held.
-//
-//go:nosplit
-func (r *AtomicRefCount) TryIncRef() bool {
-	const speculativeRef = 1 << 32
-	v := r.refCount.Add(speculativeRef)
-	if int32(v) < 0 {
-		// This object has already been freed.
-		r.refCount.Add(-speculativeRef)
-		return false
-	}
-
-	// Turn into a real reference.
-	r.refCount.Add(-speculativeRef + 1)
-	return true
-}
-
-// addWeakRef adds the given weak reference.
-func (r *AtomicRefCount) addWeakRef(w *WeakRef) {
-	r.mu.Lock()
-	r.weakRefs.PushBack(w)
-	r.mu.Unlock()
-}
-
-// dropWeakRef drops the given weak reference.
-func (r *AtomicRefCount) dropWeakRef(w *WeakRef) {
-	r.mu.Lock()
-	r.weakRefs.Remove(w)
-	r.mu.Unlock()
-}
-
-// DecRefWithDestructor decrements the object's reference count. If the
-// resulting count is negative and the destructor is not nil, then the
-// destructor will be called.
-//
-// Note that speculative references are counted here. Since they were added
-// prior to real references reaching zero, they will successfully convert to
-// real references. In other words, we see speculative references only in the
-// following case:
-//
-//	A: TryIncRef [speculative increase => sees non-negative references]
-//	B: DecRef [real decrease]
-//	A: TryIncRef [transform speculative to real]
-func (r *AtomicRefCount) DecRefWithDestructor(ctx context.Context, destroy func(context.Context)) {
-	switch v := r.refCount.Add(-1); {
-	case v < -1:
-		panic("Decrementing non-positive ref count")
-
-	case v == -1:
-		// Zap weak references. Note that at this point, all weak
-		// references are already invalid. That is, TryIncRef() will
-		// return false due to the reference count check.
-		r.mu.Lock()
-		for !r.weakRefs.Empty() {
-			w := r.weakRefs.Front()
-			// Capture the callback because w cannot be touched
-			// after it's zapped -- the owner is free it reuse it
-			// after that.
-			user := w.user
-			r.weakRefs.Remove(w)
-			w.zap()
-
-			if user != nil {
-				r.mu.Unlock()
-				user.WeakRefGone(ctx)
-				r.mu.Lock()
-			}
-		}
-		r.mu.Unlock()
-
-		// Call the destructor.
-		if destroy != nil {
-			destroy(ctx)
-		}
-	}
-}
-
-// DecRef decrements this object's reference count.
-func (r *AtomicRefCount) DecRef(ctx context.Context) {
-	r.DecRefWithDestructor(ctx, nil)
-}
-
 // OnExit is called on sandbox exit. It runs GC to enqueue refcount finalizers,
 // which check for reference leaks. There is no way to guarantee that every
 // finalizer will run before exiting, but this at least ensures that they will

pkg/refs/refcounter_state.go

@@ -1,35 +0,0 @@
-// Copyright 2018 The gVisor Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package refs
-
-// +stateify savable
-type savedReference struct {
-	obj any
-}
-
-func (w *WeakRef) saveObj() savedReference {
-	// We load the object directly, because it is typed. This will be
-	// serialized and loaded as a typed value.
-	return savedReference{w.obj.Load()}
-}
-
-func (w *WeakRef) loadObj(v savedReference) {
-	// See note above. This will be serialized and loaded typed. So we're okay
-	// as long as refs aren't changing during save and load (which they should
-	// not be).
-	//
-	// w.user is loaded before loadObj is called.
-	w.init(v.obj.(RefCounter), w.user)
-}

pkg/refs/refcounter_test.go

@@ -1,179 +0,0 @@
-// Copyright 2018 The gVisor Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package refs
-
-import (
-	"reflect"
-	"testing"
-
-	"gvisor.dev/gvisor/pkg/context"
-	"gvisor.dev/gvisor/pkg/sync"
-)
-
-type testCounter struct {
-	AtomicRefCount
-
-	// mu protects the boolean below.
-	mu sync.Mutex
-
-	// destroyed indicates whether this was destroyed.
-	destroyed bool
-}
-
-func (t *testCounter) DecRef(ctx context.Context) {
-	t.AtomicRefCount.DecRefWithDestructor(ctx, t.destroy)
-}
-
-func (t *testCounter) destroy(context.Context) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	t.destroyed = true
-}
-
-func (t *testCounter) IsDestroyed() bool {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	return t.destroyed
-}
-
-func newTestCounter() *testCounter {
-	return &testCounter{destroyed: false}
-}
-
-func TestOneRef(t *testing.T) {
-	tc := newTestCounter()
-	tc.DecRef(context.Background())
-
-	if !tc.IsDestroyed() {
-		t.Errorf("object should have been destroyed")
-	}
-}
-
-func TestTwoRefs(t *testing.T) {
-	tc := newTestCounter()
-	tc.IncRef()
-	ctx := context.Background()
-	tc.DecRef(ctx)
-	tc.DecRef(ctx)
-
-	if !tc.IsDestroyed() {
-		t.Errorf("object should have been destroyed")
-	}
-}
-
-func TestMultiRefs(t *testing.T) {
-	tc := newTestCounter()
-	tc.IncRef()
-	ctx := context.Background()
-	tc.DecRef(ctx)
-
-	tc.IncRef()
-	tc.DecRef(ctx)
-
-	tc.DecRef(ctx)
-
-	if !tc.IsDestroyed() {
-		t.Errorf("object should have been destroyed")
-	}
-}
-
-func TestWeakRef(t *testing.T) {
-	tc := newTestCounter()
-	w := NewWeakRef(tc, nil)
-	ctx := context.Background()
-
-	// Try resolving.
-	if x := w.Get(); x == nil {
-		t.Errorf("weak reference didn't resolve: expected %v, got nil", tc)
-	} else {
-		x.DecRef(ctx)
-	}
-
-	// Try resolving again.
-	if x := w.Get(); x == nil {
-		t.Errorf("weak reference didn't resolve: expected %v, got nil", tc)
-	} else {
-		x.DecRef(ctx)
-	}
-
-	// Shouldn't be destroyed yet. (Can't continue if this fails.)
-	if tc.IsDestroyed() {
-		t.Fatalf("original object destroyed earlier than expected")
-	}
-
-	// Drop the original reference.
-	tc.DecRef(ctx)
-
-	// Assert destroyed.
-	if !tc.IsDestroyed() {
-		t.Errorf("original object not destroyed as expected")
-	}
-
-	// Shouldn't be anything.
-	if x := w.Get(); x != nil {
-		t.Errorf("weak reference resolved: expected nil, got %v", x)
-	}
-}
-
-func TestWeakRefDrop(t *testing.T) {
-	tc := newTestCounter()
-	w := NewWeakRef(tc, nil)
-	ctx := context.Background()
-	w.Drop(ctx)
-
-	// Just assert the list is empty.
-	if !tc.weakRefs.Empty() {
-		t.Errorf("weak reference not dropped")
-	}
-
-	// Drop the original reference.
-	tc.DecRef(ctx)
-}
-
-type testWeakRefUser struct {
-	weakRefGone func()
-}
-
-func (u *testWeakRefUser) WeakRefGone(ctx context.Context) {
-	u.weakRefGone()
-}
-
-func TestCallback(t *testing.T) {
-	called := false
-	tc := newTestCounter()
-	var w *WeakRef
-	w = NewWeakRef(tc, &testWeakRefUser{func() {
-		called = true
-
-		// Check that the weak ref has been zapped.
-		rc := w.obj.Load().(RefCounter)
-		if v := reflect.ValueOf(rc); v != reflect.Zero(v.Type()) {
-			t.Fatalf("Callback called with non-nil ptr")
-		}
-
-		// Check that we're not holding the mutex by acquiring and
-		// releasing it.
-		tc.mu.Lock()
-		tc.mu.Unlock()
-	}})
-
-	// Drop the original reference, this must trigger the callback.
-	ctx := context.Background()
-	tc.DecRef(ctx)
-
-	if !called {
-		t.Fatalf("Callback not called")
-	}
-}

pkg/refs/refs_map.go

@@ -12,13 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package refsvfs2
+package refs
 
 import (
 	"fmt"
 
 	"gvisor.dev/gvisor/pkg/log"
-	refs_vfs1 "gvisor.dev/gvisor/pkg/refs"
 	"gvisor.dev/gvisor/pkg/sync"
 )
 
@@ -50,14 +49,14 @@ func init() {
 // LeakCheckEnabled returns whether leak checking is enabled. The following
 // functions should only be called if it returns true.
 func LeakCheckEnabled() bool {
-	mode := refs_vfs1.GetLeakMode()
-	return mode != refs_vfs1.NoLeakChecking && mode != refs_vfs1.UninitializedLeakChecking
+	mode := GetLeakMode()
+	return mode != NoLeakChecking
 }
 
 // leakCheckPanicEnabled returns whether DoLeakCheck() should panic when leaks
 // are detected.
 func leakCheckPanicEnabled() bool {
-	return refs_vfs1.GetLeakMode() == refs_vfs1.LeaksPanic
+	return GetLeakMode() == LeaksPanic
 }
 
 // Register adds obj to the live object map.
@@ -116,7 +115,7 @@ func LogDecRef(obj CheckedObject, refs int64) {
 // obj.LogRefs() should be checked before calling logEvent, in order to avoid
 // calling any text processing needed to evaluate msg.
 func logEvent(obj CheckedObject, msg string) {
-	log.Infof("[%s %p] %s:\n%s", obj.RefType(), obj, msg, refs_vfs1.FormatStack(refs_vfs1.RecordStack()))
+	log.Infof("[%s %p] %s:\n%s", obj.RefType(), obj, msg, FormatStack(RecordStack()))
 }
 
 // checkOnce makes sure that leak checking is only done once. DoLeakCheck is

pkg/refs/refs_template.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 
 	"gvisor.dev/gvisor/pkg/atomicbitops"
-	"gvisor.dev/gvisor/pkg/refsvfs2"
+	"gvisor.dev/gvisor/pkg/refs"
 )
 
 // enableLogging indicates whether reference-related events should be logged (with
@@ -64,20 +64,20 @@ type Refs struct {
 // checking.
 func (r *Refs) InitRefs() {
 	r.refCount.Store(1)
-	refsvfs2.Register(r)
+	refs.Register(r)
 }
 
-// RefType implements refsvfs2.CheckedObject.RefType.
+// RefType implements refs.CheckedObject.RefType.
 func (r *Refs) RefType() string {
 	return fmt.Sprintf("%T", obj)[1:]
 }
 
-// LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
+// LeakMessage implements refs.CheckedObject.LeakMessage.
 func (r *Refs) LeakMessage() string {
 	return fmt.Sprintf("[%s %p] reference count of %d instead of 0", r.RefType(), r, r.ReadRefs())
 }
 
-// LogRefs implements refsvfs2.CheckedObject.LogRefs.
+// LogRefs implements refs.CheckedObject.LogRefs.
 func (r *Refs) LogRefs() bool {
 	return enableLogging
 }
@@ -94,7 +94,7 @@ func (r *Refs) ReadRefs() int64 {
 func (r *Refs) IncRef() {
 	v := r.refCount.Add(1)
 	if enableLogging {
-		refsvfs2.LogIncRef(r, v)
+		refs.LogIncRef(r, v)
 	}
 	if v <= 1 {
 		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
@@ -119,7 +119,7 @@ func (r *Refs) TryIncRef() bool {
 	// Turn into a real reference.
 	v := r.refCount.Add(-speculativeRef + 1)
 	if enableLogging {
-		refsvfs2.LogTryIncRef(r, v)
+		refs.LogTryIncRef(r, v)
 	}
 	return true
 }
@@ -139,14 +139,14 @@ func (r *Refs) TryIncRef() bool {
 func (r *Refs) DecRef(destroy func()) {
 	v := r.refCount.Add(-1)
 	if enableLogging {
-		refsvfs2.LogDecRef(r, v)
+		refs.LogDecRef(r, v)
 	}
 	switch {
 	case v < 0:
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %s", r, r.RefType()))
 
 	case v == 0:
-		refsvfs2.Unregister(r)
+		refs.Unregister(r)
 		// Call the destructor.
 		if destroy != nil {
 			destroy()
@@ -156,6 +156,6 @@ func (r *Refs) DecRef(destroy func()) {
 
 func (r *Refs) afterLoad() {
 	if r.ReadRefs() > 0 {
-		refsvfs2.Register(r)
+		refs.Register(r)
 	}
 }

pkg/refsvfs2/BUILD

@@ -1,39 +0,0 @@
-# TODO(gvisor.dev/issue/1624): rename this directory/package to "refs" once VFS1
-# is gone and the current refs package can be deleted.
-load("//tools:defs.bzl", "go_library")
-load("//tools/go_generics:defs.bzl", "go_template")
-
-package(licenses = ["notice"])
-
-go_template(
-    name = "refs_template",
-    srcs = [
-        "refs_template.go",
-    ],
-    opt_consts = [
-        "enableLogging",
-    ],
-    types = [
-        "T",
-    ],
-    visibility = ["//:sandbox"],
-    deps = [
-        "//pkg/log",
-        "//pkg/refs",
-    ],
-)
-
-go_library(
-    name = "refsvfs2",
-    srcs = [
-        "refs.go",
-        "refs_map.go",
-    ],
-    visibility = ["//:sandbox"],
-    deps = [
-        "//pkg/context",
-        "//pkg/log",
-        "//pkg/refs",
-        "//pkg/sync",
-    ],
-)

pkg/refsvfs2/refs.go

@@ -1,41 +0,0 @@
-// Copyright 2020 The gVisor Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// Package refsvfs2 defines an interface for a reference-counted object.
-package refsvfs2
-
-import (
-	"gvisor.dev/gvisor/pkg/context"
-)
-
-// RefCounter is the interface to be implemented by objects that are reference
-// counted.
-type RefCounter interface {
-	// IncRef increments the reference counter on the object.
-	IncRef()
-
-	// DecRef decrements the object's reference count. Users of refs_template.Refs
-	// may specify a destructor to be called once the reference count reaches zero.
-	DecRef(ctx context.Context)
-}
-
-// TryRefCounter is like RefCounter but allow the ref increment to be tried.
-type TryRefCounter interface {
-	RefCounter
-
-	// TryIncRef attempts to increment the reference count, but may fail if all
-	// references have already been dropped, in which case it returns false. If
-	// true is returned, then a valid reference is now held on the object.
-	TryIncRef() bool
-}

pkg/sentry/fsimpl/cgroupfs/BUILD

@@ -23,7 +23,7 @@ go_template_instance(
     out = "dir_refs.go",
     package = "cgroupfs",
     prefix = "dir",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "dir",
     },
@@ -57,7 +57,6 @@ go_library(
         "//pkg/hostarch",
         "//pkg/log",
         "//pkg/refs",
-        "//pkg/refsvfs2",
         "//pkg/sentry/arch",
         "//pkg/sentry/fsimpl/kernfs",
         "//pkg/sentry/kernel",

pkg/sentry/fsimpl/devpts/BUILD

@@ -8,7 +8,7 @@ go_template_instance(
     out = "root_inode_refs.go",
     package = "devpts",
     prefix = "rootInode",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "rootInode",
     },
@@ -35,7 +35,6 @@ go_library(
         "//pkg/marshal",
         "//pkg/marshal/primitive",
         "//pkg/refs",
-        "//pkg/refsvfs2",
         "//pkg/safemem",
         "//pkg/sentry/arch",
         "//pkg/sentry/fsimpl/kernfs",

pkg/sentry/fsimpl/fuse/BUILD

@@ -20,7 +20,7 @@ go_template_instance(
     out = "inode_refs.go",
     package = "fuse",
     prefix = "inode",
-    template = "//pkg/refsvfs2:refs_template",
+    template = "//pkg/refs:refs_template",
     types = {
         "T": "inode",
     },
@@ -55,7 +55,6 @@ go_library(
         "//pkg/marshal",
         "//pkg/marshal/primitive",
         "//pkg/refs",
-        "//pkg/refsvfs2",
         "//pkg/safemem",
         "//pkg/sentry/fsimpl/devtmpfs",
         "//pkg/sentry/fsimpl/kernfs",

pkg/sentry/fsimpl/gofer/BUILD

@@ -75,7 +75,6 @@ go_library(
         "//pkg/metric",
         "//pkg/p9",
         "//pkg/refs",
-        "//pkg/refsvfs2",
         "//pkg/safemem",
         "//pkg/sentry/fsimpl/host",
         "//pkg/sentry/fsimpl/lock",

pkg/sentry/fsimpl/gofer/directory.go

@@ -25,7 +25,7 @@ import (
 	"gvisor.dev/gvisor/pkg/lisafs"
 	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/p9"
-	"gvisor.dev/gvisor/pkg/refsvfs2"
+	"gvisor.dev/gvisor/pkg/refs"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/pipe"
 	"gvisor.dev/gvisor/pkg/sentry/socket/unix/transport"
@@ -132,7 +132,7 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) {
 		mmapFD:    atomicbitops.FromInt32(-1),
 		nlink:     atomicbitops.FromUint32(2),
 	}
-	refsvfs2.Register(child)
+	refs.Register(child)
 	switch opts.mode.FileType() {
 	case linux.S_IFDIR:
 		// Nothing else needs to be done.