From b5ba47d658235e6a67d9c05a539dfbc503c6246f Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 20 Feb 2024 18:47:28 +0000 Subject: [PATCH] Backport deque memory fix (#898) * 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] * 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] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump version * Make compatible with ancient Julia versions * fixup! Make compatible with ancient Julia versions * fixup! fixup! Make compatible with ancient Julia versions --------- Signed-off-by: dependabot[bot] Co-authored-by: Kiran Pamnany Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Project.toml | 2 +- src/DataStructures.jl | 8 +++++++- src/circ_deque.jl | 2 ++ src/deque.jl | 2 ++ test/test_circ_deque.jl | 23 +++++++++++++++++++++++ test/test_deque.jl | 24 ++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 2 deletions(-) diff --git a/Project.toml b/Project.toml index 1e64c6812..1ce044127 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "DataStructures" uuid = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8" -version = "0.18.16" +version = "0.18.17" [deps] Compat = "34da2185-b29b-5c13-b0c7-acf172513d20" diff --git a/src/DataStructures.jl b/src/DataStructures.jl index c9d386f39..552aff4fc 100644 --- a/src/DataStructures.jl +++ b/src/DataStructures.jl @@ -112,4 +112,10 @@ module DataStructures include("splay_tree.jl") include("deprecations.jl") -end \ No newline at end of file + + @static if VERSION <= v"1.3" + _unsetindex!(a, i) = a + else + _unsetindex! = Base._unsetindex! + end +end diff --git a/src/circ_deque.jl b/src/circ_deque.jl index 3159d7d6a..6c03e8463 100644 --- a/src/circ_deque.jl +++ b/src/circ_deque.jl @@ -63,6 +63,7 @@ end @inline Base.@propagate_inbounds function Base.pop!(D::CircularDeque) v = last(D) + _unsetindex!(D.buffer, D.last) # see issue/884 D.n -= 1 tmp = D.last - 1 D.last = ifelse(tmp < 1, D.capacity, tmp) @@ -90,6 +91,7 @@ Remove the element at the front. """ @inline Base.@propagate_inbounds function Base.popfirst!(D::CircularDeque) v = first(D) + _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 dd6c0b503..5ae47459a 100644 --- a/src/deque.jl +++ b/src/deque.jl @@ -276,6 +276,7 @@ function Base.pop!(q::Deque{T}) where T @assert rear.back >= rear.front @inbounds x = rear.data[rear.back] + _unsetindex!(rear.data, rear.back) # see issue/884 rear.back -= 1 if rear.back < rear.front if q.nblocks > 1 @@ -301,6 +302,7 @@ function Base.popfirst!(q::Deque{T}) where T @assert head.back >= head.front @inbounds x = head.data[head.front] + _unsetindex!(head.data, head.front) # see issue/884 head.front += 1 if head.back < head.front if q.nblocks > 1 diff --git a/test/test_circ_deque.jl b/test/test_circ_deque.jl index adf3ddd6d..269d8a76c 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 + + VERSION >= v"1.3" && @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 8c35ba289..35f78ea5c 100644 --- a/test/test_deque.jl +++ b/test/test_deque.jl @@ -197,4 +197,28 @@ @test typeof(empty!(q)) === typeof(Deque{Int}()) @test isempty(q) end + + VERSION >= v"1.3" && @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