From 5451407b8c7e66a2e572d2f50c0f75e675016d35 Mon Sep 17 00:00:00 2001
From: Kiran Pamnany <kpamnany@users.noreply.github.com>
Date: Tue, 20 Feb 2024 13:02:07 -0500
Subject: [PATCH] Use `Base._unsetindex!` in `pop!` and `popfirst!` (#897)

* Bump codecov/codecov-action from 3 to 4

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3 to 4.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](https://github.com/codecov/codecov-action/compare/v3...v4)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* Use `Base._unsetindex!` in `pop!` and `popfirst!` for Deque

So that popped elements are not rooted by the deque and can be
GCed when they drop out of caller scope.

* Test Deque for leaks

* Use `Base._unsetindex!` in `pop!` and `popfirst!` for CircularDeque

So that popped elements are not rooted by the deque and can be GCed
when they drop out of caller scope.

* Test CircularDeque for leaks

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
---
 src/circ_deque.jl       |  2 ++
 src/deque.jl            |  2 ++
 test/test_circ_deque.jl | 23 +++++++++++++++++++++++
 test/test_deque.jl      | 24 ++++++++++++++++++++++++
 4 files changed, 51 insertions(+)

diff --git a/src/circ_deque.jl b/src/circ_deque.jl
index a6520e5fa..fcf303ca3 100644
--- a/src/circ_deque.jl
+++ b/src/circ_deque.jl
@@ -64,6 +64,7 @@ end
 
 @inline Base.@propagate_inbounds function Base.pop!(D::CircularDeque)
     v = last(D)
+    Base._unsetindex!(D.buffer, D.last) # see issue/884
     D.n -= 1
     tmp = D.last - 1
     D.last = ifelse(tmp < 1, D.capacity, tmp)
@@ -91,6 +92,7 @@ Remove the element at the front.
 """
 @inline Base.@propagate_inbounds function Base.popfirst!(D::CircularDeque)
     v = first(D)
+    Base._unsetindex!(D.buffer, D.first) # see issue/884
     D.n -= 1
     tmp = D.first + 1
     D.first = ifelse(tmp > D.capacity, 1, tmp)
diff --git a/src/deque.jl b/src/deque.jl
index 1ba54c131..d1b1bb163 100644
--- a/src/deque.jl
+++ b/src/deque.jl
@@ -297,6 +297,7 @@ function Base.pop!(d::Deque{T}) where T
     @assert rear.back >= rear.front
 
     @inbounds x = rear.data[rear.back]
+    Base._unsetindex!(rear.data, rear.back) # see issue/884
     rear.back -= 1
     if rear.back < rear.front
         if d.nblocks > 1
@@ -322,6 +323,7 @@ function Base.popfirst!(d::Deque{T}) where T
     @assert head.back >= head.front
 
     @inbounds x = head.data[head.front]
+    Base._unsetindex!(head.data, head.front) # see issue/884
     head.front += 1
     if head.back < head.front
         if d.nblocks > 1
diff --git a/test/test_circ_deque.jl b/test/test_circ_deque.jl
index adf3ddd6d..b0bd30dca 100644
--- a/test/test_circ_deque.jl
+++ b/test/test_circ_deque.jl
@@ -81,6 +81,29 @@
         for i in 1:5 push!(D, i) end
         @test collect([i for i in D]) == collect(1:5)
     end
+
+    @testset "pop! and popfirst! do not leak" begin
+        D = CircularDeque{String}(5)
+
+        @testset "pop! doesn't leak" begin
+            push!(D,"foo")
+            push!(D,"bar")
+            ss2 = Base.summarysize(D)
+            pop!(D)
+            GC.gc(true)
+            ss1 = Base.summarysize(D)
+            @test ss1 < ss2
+        end
+        @testset "popfirst! doesn't leak" begin
+            push!(D,"baz")
+            push!(D,"bug")
+            ss2 = Base.summarysize(D)
+            popfirst!(D)
+            GC.gc(true)
+            ss1 = Base.summarysize(D)
+            @test ss1 < ss2
+        end
+    end
 end
 
 nothing
diff --git a/test/test_deque.jl b/test/test_deque.jl
index d5891fee0..ada4912d5 100644
--- a/test/test_deque.jl
+++ b/test/test_deque.jl
@@ -212,4 +212,28 @@
         @test last(q) == 3
         @test num_blocks(q) == 1
     end
+
+    @testset "pop! and popfirst! don't leak" begin
+        q = Deque{String}(5)
+        GC.gc(true)
+
+        @testset "pop! doesn't leak" begin
+            push!(q,"foo")
+            push!(q,"bar")
+            ss2 = Base.summarysize(q.head)
+            pop!(q)
+            GC.gc(true)
+            ss1 = Base.summarysize(q.head)
+            @test ss1 < ss2
+        end
+        @testset "popfirst! doesn't leak" begin
+            push!(q,"baz")
+            push!(q,"bug")
+            ss2 = Base.summarysize(q.head)
+            popfirst!(q)
+            GC.gc(true)
+            ss1 = Base.summarysize(q.head)
+            @test ss1 < ss2
+        end
+    end
 end # @testset Deque