Skip to content

Commit

Permalink
Use Base._unsetindex! in pop! and popfirst! (#897)
Browse files Browse the repository at this point in the history
* 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](codecov/codecov-action@v3...v4)

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

Signed-off-by: dependabot[bot] <[email protected]>

* 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] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
2 people authored and nickrobinson251 committed Feb 20, 2024
1 parent 685f7bc commit 6a0c6e7
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/circ_deque.jl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,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)
Expand Down Expand Up @@ -90,6 +91,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)
Expand Down
2 changes: 2 additions & 0 deletions src/deque.jl
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ function Base.pop!(q::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 q.nblocks > 1
Expand All @@ -301,6 +302,7 @@ function Base.popfirst!(q::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 q.nblocks > 1
Expand Down
23 changes: 23 additions & 0 deletions test/test_circ_deque.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
24 changes: 24 additions & 0 deletions test/test_deque.jl
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,28 @@
@test typeof(empty!(q)) === typeof(Deque{Int}())
@test isempty(q)
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

0 comments on commit 6a0c6e7

Please sign in to comment.