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 <[email protected]>
Reviewed-by: Eli Bendersky <[email protected]>

Author: Deleplace

slices/slices.go

@@ -209,25 +209,37 @@ func Insert[S ~[]E, E any](s S, i int, v ...E) S {
 	return s
 }
 
+// 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
+	}
+}
+
 // 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
+// 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 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.
+// 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] // bounds check
+	_ = s[i:j:len(s)] // bounds check
 
-	return append(s[:i], s[j:]...)
+	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 @@ func DeleteFunc[S ~[]E, E any](s S, del func(E) bool) S {
 			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 @@ func Replace[S ~[]E, E any](s S, i, j int, v ...E) S {
 		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 @@ func Clone[S ~[]E, E any](s S) S {
 // 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 @@ func Compact[S ~[]E, E comparable](s S) S {
 			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 @@ func CompactFunc[S ~[]E, E any](s S, eq func(E, E) bool) S {
 			i++
 		}
 	}
+	clearSlice(s[i:]) // zero/nil out the obsolete elements, for GC
 	return s[:i]
 }
 

slices/slices_test.go

@@ -654,6 +654,39 @@ func TestDeletePanics(t *testing.T) {
 	}
 }
 
+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 TestCompactFunc(t *testing.T) {
 	}
 }
 
+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 TestReplacePanics(t *testing.T) {
 	}
 }
 
+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)