diff --git a/CHANGELOG.md b/CHANGELOG.md index 1838245d..10b4b911 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - The minimum supported julia version is increased to 1.6. ([#328]) +- `test_persistent_tasks` now reports a precompilation failure as a precompilation + error instead of misclassifying it as a persistent task. The default `tmax` is + also raised to 30 seconds (still configurable) to reduce false positives when a + package without persistent tasks is merely slow to shut down. ([#315]) ## Version [v0.8.16] - 2026-06-05 @@ -350,6 +354,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#297]: https://github.com/JuliaTesting/Aqua.jl/issues/297 [#309]: https://github.com/JuliaTesting/Aqua.jl/issues/309 [#313]: https://github.com/JuliaTesting/Aqua.jl/issues/313 +[#315]: https://github.com/JuliaTesting/Aqua.jl/issues/315 [#319]: https://github.com/JuliaTesting/Aqua.jl/issues/319 [#322]: https://github.com/JuliaTesting/Aqua.jl/issues/322 [#328]: https://github.com/JuliaTesting/Aqua.jl/issues/328 diff --git a/docs/src/persistent_tasks.md b/docs/src/persistent_tasks.md index 5fe3e107..2de1aaa1 100644 --- a/docs/src/persistent_tasks.md +++ b/docs/src/persistent_tasks.md @@ -84,8 +84,18 @@ end) This test works by launching a Julia process that tries to precompile a dummy package similar to `PkgB` above, modified to signal back to Aqua when -`PkgA` has finished loading. The test fails if the gap between loading `PkgA` -and finishing precompilation exceeds time `tmax`. +`PkgA` has finished loading. + +Aqua waits — without any time limit — for that signal, so slow precompilation of +the dependencies never affects the result. Once `PkgA` has loaded, a persistent +`Task` shows up as the precompilation process being unable to exit: it hangs +indefinitely. A package without persistent tasks always exits eventually, so Aqua +waits up to `tmax` seconds for its shutdown before reporting a failure. If a +package you know to be free of persistent tasks is misreported, increase `tmax`. + +If precompilation instead fails outright (for example because a dependency +cannot be precompiled), that is reported as a precompilation error rather than a +persistent task, so the failure message points at the real cause. ## How to fix failing packages diff --git a/src/persistent_tasks.jl b/src/persistent_tasks.jl index 2daef554..d0f66a37 100644 --- a/src/persistent_tasks.jl +++ b/src/persistent_tasks.jl @@ -18,9 +18,11 @@ On Julia version 1.9 and before, this test always succeeds. # Keyword Arguments - `broken::Bool = false`: If true, it uses `@test_broken` instead of `@test`. -- `tmax::Real = 5`: the maximum time (in seconds) to wait after loading the - package before forcibly shutting down the precompilation process (triggering - a test failure). +- `tmax::Real = 30`: the maximum time (in seconds) to wait for the + precompilation process to exit *after* `package` has finished loading. Only + this shutdown counts against `tmax`, not the time spent loading the + dependencies. A persistent `Task` blocks the exit indefinitely, so if a + package free of persistent tasks is misreported, increase `tmax`. - `expr::Expr = quote end`: An expression to run in the precompile package. !!! note @@ -44,7 +46,7 @@ function test_persistent_tasks(package::Module; kwargs...) test_persistent_tasks(PkgId(package); kwargs...) end -function has_persistent_tasks(package::PkgId; expr::Expr = quote end, tmax = 10) +function has_persistent_tasks(package::PkgId; expr::Expr = quote end, tmax = 30) root_project_path, found = root_project_toml(package) found || error("Unable to locate Project.toml") return !precompile_wrapper(root_project_path, tmax, expr) @@ -118,25 +120,44 @@ end code = """touch("$(escape_string(statusfile))")""" `$(Base.julia_cmd()) -e $code` else - `$(Base.julia_cmd()) --project=$wrapperdir -e 'push!(LOAD_PATH, "@stdlib"); using Pkg; Pkg.precompile(; io = devnull)'` + `$(Base.julia_cmd()) --project=$wrapperdir -e 'push!(LOAD_PATH, "@stdlib"); using Pkg; Pkg.precompile()'` end - cmd = pipeline(cmd; stdout, stderr) + # Capture the subprocess's stderr so a genuine precompilation error can be + # reported on its own terms instead of masquerading as a persistent task. + errlog = joinpath(wrapperdir, "precompile-stderr.log") + cmd = pipeline(cmd; stdout = devnull, stderr = errlog) proc = run(cmd; wait = false)::Base.Process - while !isfile(statusfile) && process_running(proc) - sleep(0.5) - end + + # Phase 1 (unbounded): wait for the package to finish loading. The wrapper + # writes `statusfile` once `using $pkgname` (and any `expr`) has run. Slow + # precompilation of the dependencies only prolongs this phase. + timedwait(() -> isfile(statusfile) || !process_running(proc), Inf; pollint = 0.5) if !isfile(statusfile) - @error "Unexpected error: $statusfile was not created, but precompilation exited" - return false - end - # Check whether precompilation finishes in the required time - t = time() - while process_running(proc) && time() - t < tmax - sleep(0.1) + # The process exited before the package finished loading: a + # precompilation failure, not a persistent task. + wait(proc) + error( + "Loading `$pkgname` for the persistent-task check failed before " * + "precompilation completed (process exited with code " * + "$(proc.exitcode), signal $(proc.termsignal)). This indicates a " * + "precompilation error, not a persistent task. Captured output:\n\n" * + (isfile(errlog) ? read(errlog, String) : ""), + ) end + + # Phase 2 (bounded by `tmax`): the package loaded cleanly. A persistent task + # keeps the process from exiting, so it hangs indefinitely. A healthy package + # exits once its shutdown finishes, so allow up to `tmax` seconds for it. + timedwait(() -> !process_running(proc), tmax; pollint = 0.1) success = !process_running(proc) if !success + @warn( + "Loading `$pkgname` prevented the precompilation process from " * + "exiting within $tmax seconds, which usually means a persistent " * + "task is still running. If `$pkgname` is free of persistent tasks, " * + "re-run with a larger `tmax` to give its shutdown more time." + ) # SIGKILL to prevent julia from printing the SIG 15 handler, which can # misleadingly look like it's caused by an issue in the user's program. kill(proc, Base.SIGKILL) diff --git a/test/pkgs/PersistentTasks/FailsToPrecompile/Project.toml b/test/pkgs/PersistentTasks/FailsToPrecompile/Project.toml new file mode 100644 index 00000000..a1d82983 --- /dev/null +++ b/test/pkgs/PersistentTasks/FailsToPrecompile/Project.toml @@ -0,0 +1,2 @@ +name = "FailsToPrecompile" +uuid = "f9ce74f8-1127-4e05-8a6a-300e168e70af" diff --git a/test/pkgs/PersistentTasks/FailsToPrecompile/src/FailsToPrecompile.jl b/test/pkgs/PersistentTasks/FailsToPrecompile/src/FailsToPrecompile.jl new file mode 100644 index 00000000..7771db5e --- /dev/null +++ b/test/pkgs/PersistentTasks/FailsToPrecompile/src/FailsToPrecompile.jl @@ -0,0 +1,7 @@ +module FailsToPrecompile + +# Fail during precompilation so the persistent-task check hits its +# precompilation-error branch instead of loading the package successfully. +error("Intentional precompilation failure for testing Aqua's persistent-task check") + +end diff --git a/test/test_persistent_tasks.jl b/test/test_persistent_tasks.jl index 1339f159..0dc14cb9 100644 --- a/test/test_persistent_tasks.jl +++ b/test/test_persistent_tasks.jl @@ -21,12 +21,23 @@ end @test result == [] if Base.VERSION >= v"1.10-" - @test Aqua.has_persistent_tasks(getid("PersistentTask")) + @test Aqua.has_persistent_tasks(getid("PersistentTask"); tmax = 2) - result = Aqua.find_persistent_tasks_deps(getid("UsesBoth")) + result = Aqua.find_persistent_tasks_deps(getid("UsesBoth"); tmax = 2) @test result == ["PersistentTask"] end - filter!(str -> !occursin("PersistentTasks", str), LOAD_PATH) + filter!(!occursin("PersistentTasks"), LOAD_PATH) +end + +@testset "precompilation failure is reported as an error" begin + if Base.VERSION >= v"1.10-" + # A package that fails to precompile must be reported as a + # precompilation error rather than misclassified as a persistent task. + @test_throws "precompilation error" Aqua.has_persistent_tasks( + getid("FailsToPrecompile"), + ) + end + filter!(!occursin("PersistentTasks"), LOAD_PATH) end @testset "test_persistent_tasks(expr)" begin @@ -37,11 +48,15 @@ end fetch(Threads.@spawn nothing) end, ) - @test Aqua.has_persistent_tasks(getid("TransientTask"), expr = quote - Threads.@spawn while true - sleep(0.5) - end - end) + @test Aqua.has_persistent_tasks( + getid("TransientTask"), + tmax = 2, + expr = quote + Threads.@spawn while true + sleep(0.5) + end + end, + ) end end