From c6abf148c5c6a78c03ebedf55a8fd64b3bbf4210 Mon Sep 17 00:00:00 2001 From: Min-Gu Yoo Date: Wed, 1 Jul 2026 10:51:22 -0700 Subject: [PATCH] (fix): disable_auto_manage! clears pending requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit disable_auto_manage! only closed the Timer, leaving each pool's _compact_requested / _trim_requested flags set. A flag set by the last sweep before disable would then trigger one more compaction/trim cycle at the next @with_pool entry — an unexpected allocation after the user explicitly turned auto-manage off. Clear all live pools' flags (and reset the trim tick) in the timer-stop chokepoint, under _AUTO_MANAGE_LOCK so stop-and-clear is atomic against a concurrent sweep. Add a regression test. --- src/auto_manage.jl | 21 +++++++++++++++++++++ test/test_auto_manage.jl | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/auto_manage.jl b/src/auto_manage.jl index ff220c36..6462da6e 100644 --- a/src/auto_manage.jl +++ b/src/auto_manage.jl @@ -225,12 +225,33 @@ end _stop_auto_manage_timer!() = (lock(_stop_auto_manage_timer_unlocked!, _AUTO_MANAGE_LOCK); nothing) +# Drop every live pool's pending maintenance request and reset the trim cadence. Called from +# the timer-stop path (below) so that once the Timer is gone, no stale flag set by the last +# sweep can still trigger one post-disable `_run_auto_manage!` at a later `@with_pool` entry. +# Runs under `_AUTO_MANAGE_LOCK` (the caller holds it), the same lock the sweep takes, so the +# stop-and-clear is atomic against a concurrent sweep. Dead WeakRefs are skipped (not pruned — +# the next sweep prunes them); the `_trim_requested` clear is guarded for backends lacking it. +function _clear_auto_manage_requests_unlocked!() + for ref in _AUTO_MANAGE_REGISTRY + pool = ref.value + pool === nothing && continue + p = pool::AbstractArrayPool + @atomic :monotonic p._compact_requested = false + if hasfield(typeof(p), :_trim_requested) + @atomic :monotonic p._trim_requested = false + end + end + _AUTO_MANAGE_TICK[] = 0 + return nothing +end + function _stop_auto_manage_timer_unlocked!() t = _AUTO_MANAGE_TIMER[] if t !== nothing close(t) _AUTO_MANAGE_TIMER[] = nothing end + _clear_auto_manage_requests_unlocked!() # no stale flag outlives the timer return nothing end diff --git a/test/test_auto_manage.jl b/test/test_auto_manage.jl index 777de58a..d182e266 100644 --- a/test/test_auto_manage.jl +++ b/test/test_auto_manage.jl @@ -151,6 +151,29 @@ AAP.disable_auto_manage!() # stop the __init__-started timer for deterministic _clear_registry!() end + @testset "disable! clears pending requests so no post-disable auto-manage runs" begin + # Regression: `disable_auto_manage!` must not just stop the Timer — it must also drop + # any flag the last sweep already set, or the NEXT `@with_pool` entry would run one + # more compaction/trim AFTER the user explicitly turned auto-manage off (surprising an + # allocation on a hot path someone disabled precisely to keep zero-alloc). + _clear_registry!() + AAP.disable_auto_manage!() + pool = _inactive_bloated(1_000_000, 100) + AAP.register_auto_manage!(pool) + + AAP._auto_manage_sweep!(nothing) # last pre-disable sweep sets the flag + @test (@atomic pool._compact_requested) == true + + AAP.disable_auto_manage!() # explicit off → must also clear flags + @test AAP.auto_manage_enabled() == false + @test (@atomic pool._compact_requested) == false # stale request dropped + + AAP._maybe_auto_manage!(pool) # a later scope entry now no-ops + @test (@atomic pool._compact_requested) == false + @test _cap(pool.float64.vectors[1]) >= 1_000_000 # NOT compacted post-disable + _clear_registry!() + end + @testset "enable! replaces an existing timer (closes the old one)" begin AAP.disable_auto_manage!() AAP.enable_auto_manage!(compact_interval = 0.1)