From 02bc700862a1677dbf787b905257885154f1f6ec Mon Sep 17 00:00:00 2001 From: Kristoffer Carlsson Date: Mon, 29 Jan 2024 12:51:55 +0100 Subject: [PATCH] Extensions: make loading of extensions independent of what packages are in the sysimage (#52841) When triggers of extension are in the sysimage it is easy to end up with cycles in package loading. Say we have a package A with exts BExt and CExt and say that B and C is in the sysimage. - Upon loading A, we will immidiately start to precompile BExt (because the trigger B is "loaded" by virtue of being in the sysimage). - BExt will load A which will cause CExt to start precompiling (again because C is in the sysimage). - CExt will load A which will now cause BExt to start loading and we get a cycle. This is fixed in this PR by instead of looking at what modules are loaded, we look at what modules are actually `require`d and only use that to drive the loading of extensions. Fixes https://github.com/JuliaLang/julia/issues/52132. (cherry picked from commit 08d229f4a7cb0c3ef5becddd1b3bc4f8f178b8e4) --- base/Base.jl | 1 + base/loading.jl | 10 +++++++++- test/loading.jl | 18 ++++++++++++++++++ .../HasDepWithExtensions.jl/Manifest.toml | 9 +++++++-- .../Extensions/HasExtensions.jl/Project.toml | 2 ++ .../HasExtensions.jl/ext/LinearAlgebraExt.jl | 3 +++ 6 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl diff --git a/base/Base.jl b/base/Base.jl index 6ed423a799e4e..5e5588465bdf7 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -595,6 +595,7 @@ function __init__() init_load_path() init_active_project() append!(empty!(_sysimage_modules), keys(loaded_modules)) + empty!(explicit_loaded_modules) if haskey(ENV, "JULIA_MAX_NUM_PRECOMPILE_FILES") MAX_NUM_PRECOMPILE_FILES[] = parse(Int, ENV["JULIA_MAX_NUM_PRECOMPILE_FILES"]) end diff --git a/base/loading.jl b/base/loading.jl index b652e56c922e0..c1972907b02bb 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1275,7 +1275,7 @@ function _insert_extension_triggers(parent::PkgId, extensions::Dict{String, Any} # TODO: Better error message if this lookup fails? uuid_trigger = UUID(weakdeps[trigger]::String) trigger_id = PkgId(uuid_trigger, trigger) - if !haskey(Base.loaded_modules, trigger_id) || haskey(package_locks, trigger_id) + if !haskey(explicit_loaded_modules, trigger_id) || haskey(package_locks, trigger_id) trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, trigger_id) push!(trigger1, gid) else @@ -1821,6 +1821,11 @@ function __require_prelocked(uuidkey::PkgId, env=nothing) REPL_MODULE_REF[] = newm end else + m = get(loaded_modules, uuidkey, nothing) + if m !== nothing + explicit_loaded_modules[uuidkey] = m + run_package_callbacks(uuidkey) + end newm = root_module(uuidkey) end return newm @@ -1835,6 +1840,8 @@ PkgOrigin() = PkgOrigin(nothing, nothing, nothing) const pkgorigins = Dict{PkgId,PkgOrigin}() const loaded_modules = Dict{PkgId,Module}() +# Emptied on Julia start +const explicit_loaded_modules = Dict{PkgId,Module}() const loaded_modules_order = Vector{Module}() const module_keys = IdDict{Module,PkgId}() # the reverse @@ -1858,6 +1865,7 @@ root_module_key(m::Module) = @lock require_lock module_keys[m] end push!(loaded_modules_order, m) loaded_modules[key] = m + explicit_loaded_modules[key] = m module_keys[m] = key end nothing diff --git a/test/loading.jl b/test/loading.jl index 394c13c5f2962..e5f508b9bb55f 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1083,6 +1083,24 @@ end cmd_proj_ext = addenv(cmd_proj_ext, "JULIA_LOAD_PATH" => join([joinpath(proj, "HasExtensions.jl"), joinpath(proj, "EnvWithDeps")], sep)) run(cmd_proj_ext) end + + # Sysimage extensions + # The test below requires that LinearAlgebra is in the sysimage and that it has not been loaded yet. + # if it gets moved out, this test will need to be updated. + # We run this test in a new process so we are not vulnerable to a previous test having loaded LinearAlgebra + sysimg_ext_test_code = """ + uuid_key = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra") + Base.in_sysimage(uuid_key) || error("LinearAlgebra not in sysimage") + haskey(Base.explicit_loaded_modules, uuid_key) && error("LinearAlgebra already loaded") + using HasExtensions + Base.get_extension(HasExtensions, :LinearAlgebraExt) === nothing || error("unexpectedly got an extension") + using LinearAlgebra + haskey(Base.explicit_loaded_modules, uuid_key) || error("LinearAlgebra not loaded") + Base.get_extension(HasExtensions, :LinearAlgebraExt) isa Module || error("expected extension to load") + """ + cmd = `$(Base.julia_cmd()) --startup-file=no -e $sysimg_ext_test_code` + cmd = addenv(cmd, "JULIA_LOAD_PATH" => join([proj, "@stdlib"], sep)) + run(cmd) finally try rm(depot_path, force=true, recursive=true) diff --git a/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml b/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml index 52542fc822094..15f068f250ce3 100644 --- a/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml +++ b/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml @@ -1,6 +1,6 @@ # This file is machine-generated - editing it directly is not advised -julia_version = "1.10.0-DEV" +julia_version = "1.10.0" manifest_format = "2.0" project_hash = "d523b3401f72a1ed34b7b43749fd2655c6b78542" @@ -19,11 +19,16 @@ version = "0.1.0" path = "../HasExtensions.jl" uuid = "4d3288b3-3afc-4bb6-85f3-489fffe514c8" version = "0.1.0" -weakdeps = ["ExtDep", "ExtDep2"] [deps.HasExtensions.extensions] Extension = "ExtDep" ExtensionFolder = ["ExtDep", "ExtDep2"] + LinearAlgebraExt = "LinearAlgebra" + + [deps.HasExtensions.weakdeps] + ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c" + ExtDep2 = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d" + LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" [[deps.SomePackage]] path = "../SomePackage" diff --git a/test/project/Extensions/HasExtensions.jl/Project.toml b/test/project/Extensions/HasExtensions.jl/Project.toml index 72577de36d65d..a5f9bb1e42d29 100644 --- a/test/project/Extensions/HasExtensions.jl/Project.toml +++ b/test/project/Extensions/HasExtensions.jl/Project.toml @@ -5,7 +5,9 @@ version = "0.1.0" [weakdeps] ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c" ExtDep2 = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d" +LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" [extensions] Extension = "ExtDep" ExtensionFolder = ["ExtDep", "ExtDep2"] +LinearAlgebraExt = "LinearAlgebra" diff --git a/test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl b/test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl new file mode 100644 index 0000000000000..19f87cb849417 --- /dev/null +++ b/test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl @@ -0,0 +1,3 @@ +module LinearAlgebraExt + +end