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] <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>
  • Loading branch information
kpamnany and dependabot[bot] committed Feb 20, 2024
1 parent 1b1d67b commit 5451407
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 @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions src/deque.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
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 @@ -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

0 comments on commit 5451407

Please sign in to comment.