slices: zero the slice elements discarded by Delete, DeleteFunc, Compact, CompactFunc, Replace.
Backport from stdlib: to avoid memory leaks in slices that contain pointers, clear the elements between the new length and the original length.
Fixes golang/go#63393
Change-Id: I38bf64b27619d8067f2e95ce3c7952ec95ca55b8
Reviewed-on: https://go-review.googlesource.com/c/exp/+/543335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Eli Bendersky <eliben@google.com>
diff --git a/slices/slices.go b/slices/slices.go
index 5e8158b..46ceac3 100644
--- a/slices/slices.go
+++ b/slices/slices.go
@@ -209,25 +209,37 @@
return s
}
-// Delete removes the elements s[i:j] from s, returning the modified slice.
-// Delete panics if s[i:j] is not a valid slice of s.
-// Delete is O(len(s)-j), so if many items must be deleted, it is better to
-// make a single call deleting them all together than to delete one at a time.
-// Delete might not modify the elements s[len(s)-(j-i):len(s)]. If those
-// elements contain pointers you might consider zeroing those elements so that
-// objects they reference can be garbage collected.
-func Delete[S ~[]E, E any](s S, i, j int) S {
- _ = s[i:j] // bounds check
+// clearSlice sets all elements up to the length of s to the zero value of E.
+// We may use the builtin clear func instead, and remove clearSlice, when upgrading
+// to Go 1.21+.
+func clearSlice[S ~[]E, E any](s S) {
+ var zero E
+ for i := range s {
+ s[i] = zero
+ }
+}
- return append(s[:i], s[j:]...)
+// Delete removes the elements s[i:j] from s, returning the modified slice.
+// Delete panics if j > len(s) or s[i:j] is not a valid slice of s.
+// Delete is O(len(s)-i), so if many items must be deleted, it is better to
+// make a single call deleting them all together than to delete one at a time.
+// Delete zeroes the elements s[len(s)-(j-i):len(s)].
+func Delete[S ~[]E, E any](s S, i, j int) S {
+ _ = s[i:j:len(s)] // bounds check
+
+ if i == j {
+ return s
+ }
+
+ oldlen := len(s)
+ s = append(s[:i], s[j:]...)
+ clearSlice(s[len(s):oldlen]) // zero/nil out the obsolete elements, for GC
+ return s
}
// DeleteFunc removes any elements from s for which del returns true,
// returning the modified slice.
-// When DeleteFunc removes m elements, it might not modify the elements
-// s[len(s)-m:len(s)]. If those elements contain pointers you might consider
-// zeroing those elements so that objects they reference can be garbage
-// collected.
+// DeleteFunc zeroes the elements between the new length and the original length.
func DeleteFunc[S ~[]E, E any](s S, del func(E) bool) S {
i := IndexFunc(s, del)
if i == -1 {
@@ -240,11 +252,13 @@
i++
}
}
+ clearSlice(s[i:]) // zero/nil out the obsolete elements, for GC
return s[:i]
}
// Replace replaces the elements s[i:j] by the given v, and returns the
// modified slice. Replace panics if s[i:j] is not a valid slice of s.
+// When len(v) < (j-i), Replace zeroes the elements between the new length and the original length.
func Replace[S ~[]E, E any](s S, i, j int, v ...E) S {
_ = s[i:j] // verify that i:j is a valid subslice
@@ -272,6 +286,7 @@
if i+len(v) != j {
copy(r[i+len(v):], s[j:])
}
+ clearSlice(s[tot:]) // zero/nil out the obsolete elements, for GC
return r
}
@@ -345,9 +360,7 @@
// This is like the uniq command found on Unix.
// Compact modifies the contents of the slice s and returns the modified slice,
// which may have a smaller length.
-// When Compact discards m elements in total, it might not modify the elements
-// s[len(s)-m:len(s)]. If those elements contain pointers you might consider
-// zeroing those elements so that objects they reference can be garbage collected.
+// Compact zeroes the elements between the new length and the original length.
func Compact[S ~[]E, E comparable](s S) S {
if len(s) < 2 {
return s
@@ -361,11 +374,13 @@
i++
}
}
+ clearSlice(s[i:]) // zero/nil out the obsolete elements, for GC
return s[:i]
}
// CompactFunc is like [Compact] but uses an equality function to compare elements.
// For runs of elements that compare equal, CompactFunc keeps the first one.
+// CompactFunc zeroes the elements between the new length and the original length.
func CompactFunc[S ~[]E, E any](s S, eq func(E, E) bool) S {
if len(s) < 2 {
return s
@@ -379,6 +394,7 @@
i++
}
}
+ clearSlice(s[i:]) // zero/nil out the obsolete elements, for GC
return s[:i]
}
diff --git a/slices/slices_test.go b/slices/slices_test.go
index 7371bdb..38bbede 100644
--- a/slices/slices_test.go
+++ b/slices/slices_test.go
@@ -654,6 +654,39 @@
}
}
+func TestDeleteClearTail(t *testing.T) {
+ mem := []*int{new(int), new(int), new(int), new(int), new(int), new(int)}
+ s := mem[0:5] // there is 1 element beyond len(s), within cap(s)
+
+ s = Delete(s, 2, 4)
+
+ if mem[3] != nil || mem[4] != nil {
+ // Check that potential memory leak is avoided
+ t.Errorf("Delete: want nil discarded elements, got %v, %v", mem[3], mem[4])
+ }
+ if mem[5] == nil {
+ t.Errorf("Delete: want unchanged elements beyond original len, got nil")
+ }
+}
+
+func TestDeleteFuncClearTail(t *testing.T) {
+ mem := []*int{new(int), new(int), new(int), new(int), new(int), new(int)}
+ *mem[2], *mem[3] = 42, 42
+ s := mem[0:5] // there is 1 element beyond len(s), within cap(s)
+
+ s = DeleteFunc(s, func(i *int) bool {
+ return i != nil && *i == 42
+ })
+
+ if mem[3] != nil || mem[4] != nil {
+ // Check that potential memory leak is avoided
+ t.Errorf("DeleteFunc: want nil discarded elements, got %v, %v", mem[3], mem[4])
+ }
+ if mem[5] == nil {
+ t.Errorf("DeleteFunc: want unchanged elements beyond original len, got nil")
+ }
+}
+
func TestClone(t *testing.T) {
s1 := []int{1, 2, 3}
s2 := Clone(s1)
@@ -757,6 +790,53 @@
}
}
+func TestCompactClearTail(t *testing.T) {
+ one, two, three, four := 1, 2, 3, 4
+ mem := []*int{&one, &one, &two, &two, &three, &four}
+ s := mem[0:5] // there is 1 element beyond len(s), within cap(s)
+ copy := Clone(s)
+
+ s = Compact(s)
+
+ if want := []*int{&one, &two, &three}; !Equal(s, want) {
+ t.Errorf("Compact(%v) = %v, want %v", copy, s, want)
+ }
+
+ if mem[3] != nil || mem[4] != nil {
+ // Check that potential memory leak is avoided
+ t.Errorf("Compact: want nil discarded elements, got %v, %v", mem[3], mem[4])
+ }
+ if mem[5] != &four {
+ t.Errorf("Compact: want unchanged element beyond original len, got %v", mem[5])
+ }
+}
+
+func TestCompactFuncClearTail(t *testing.T) {
+ a, b, c, d, e, f := 1, 1, 2, 2, 3, 4
+ mem := []*int{&a, &b, &c, &d, &e, &f}
+ s := mem[0:5] // there is 1 element beyond len(s), within cap(s)
+ copy := Clone(s)
+
+ s = CompactFunc(s, func(x, y *int) bool {
+ if x == nil || y == nil {
+ return x == y
+ }
+ return *x == *y
+ })
+
+ if want := []*int{&a, &c, &e}; !Equal(s, want) {
+ t.Errorf("CompactFunc(%v) = %v, want %v", copy, s, want)
+ }
+
+ if mem[3] != nil || mem[4] != nil {
+ // Check that potential memory leak is avoided
+ t.Errorf("CompactFunc: want nil discarded elements, got %v, %v", mem[3], mem[4])
+ }
+ if mem[5] != &f {
+ t.Errorf("CompactFunc: want unchanged elements beyond original len, got %v", mem[5])
+ }
+}
+
func BenchmarkCompactFunc_Large(b *testing.B) {
type Large [4 * 1024]byte
@@ -922,6 +1002,56 @@
}
}
+func TestReplaceGrow(t *testing.T) {
+ // When Replace needs to allocate a new slice, we want the original slice
+ // to not be changed.
+ a, b, c, d, e, f := 1, 2, 3, 4, 5, 6
+ mem := []*int{&a, &b, &c, &d, &e, &f}
+ memcopy := Clone(mem)
+ s := mem[0:5] // there is 1 element beyond len(s), within cap(s)
+ copy := Clone(s)
+ original := s
+
+ // The new elements don't fit within cap(s), so Replace will allocate.
+ z := 99
+ s = Replace(s, 1, 3, &z, &z, &z, &z)
+
+ if want := []*int{&a, &z, &z, &z, &z, &d, &e}; !Equal(s, want) {
+ t.Errorf("Replace(%v, 1, 3, %v, %v, %v, %v) = %v, want %v", copy, &z, &z, &z, &z, s, want)
+ }
+
+ if !Equal(original, copy) {
+ t.Errorf("original slice has changed, got %v, want %v", original, copy)
+ }
+
+ if !Equal(mem, memcopy) {
+ // Changing the original tail s[len(s):cap(s)] is unwanted
+ t.Errorf("original backing memory has changed, got %v, want %v", mem, memcopy)
+ }
+}
+
+func TestReplaceClearTail(t *testing.T) {
+ a, b, c, d, e, f := 1, 2, 3, 4, 5, 6
+ mem := []*int{&a, &b, &c, &d, &e, &f}
+ s := mem[0:5] // there is 1 element beyond len(s), within cap(s)
+ copy := Clone(s)
+
+ y, z := 8, 9
+ s = Replace(s, 1, 4, &y, &z)
+
+ if want := []*int{&a, &y, &z, &e}; !Equal(s, want) {
+ t.Errorf("Replace(%v) = %v, want %v", copy, s, want)
+ }
+
+ if mem[4] != nil {
+ // Check that potential memory leak is avoided
+ t.Errorf("Replace: want nil discarded element, got %v", mem[4])
+ }
+ if mem[5] != &f {
+ t.Errorf("Replace: want unchanged elements beyond original len, got %v", mem[5])
+ }
+}
+
func TestReplaceOverlap(t *testing.T) {
const N = 10
a := make([]int, N)
