Skip to content

WIP: improve some cases of loading extensions in the presence of packages in the sysimage. #54750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,7 @@ function _insert_extension_triggers(parent::PkgId, extensions::Dict{String, Any}
# TODO: Better error message if this lookup fails?
uuid_trigger = UUID(totaldeps[trigger]::String)
trigger_id = PkgId(uuid_trigger, trigger)
if !haskey(explicit_loaded_modules, trigger_id) || haskey(package_locks, trigger_id)
if !in(trigger_id, explicit_loaded_modules) || haskey(package_locks, trigger_id)
trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, trigger_id)
push!(trigger1, gid)
else
Expand Down Expand Up @@ -2148,6 +2148,8 @@ function require(into::Module, mod::Symbol)
end
end

const require_map = Dict{PkgId, Set{PkgId}}()

function __require(into::Module, mod::Symbol)
@lock require_lock begin
LOADING_CACHE[] = LoadingCache()
Expand Down Expand Up @@ -2192,7 +2194,13 @@ function __require(into::Module, mod::Symbol)
path = binpack(uuidkey)
push!(_require_dependencies, (into, path, UInt64(0), UInt32(0), 0.0))
end
return _require_prelocked(uuidkey, env)
push!(get!(Set{PkgId}, require_map, PkgId(into)), uuidkey)
try
return _require_prelocked(uuidkey, env)
catch
delete!(require_map, PkgId(into))
rethrow()
end
finally
LOADING_CACHE[] = nothing
end
Expand Down Expand Up @@ -2268,12 +2276,11 @@ function __require_prelocked(uuidkey::PkgId, env=nothing)
end
insert_extension_triggers(uuidkey)
# After successfully loading, notify downstream consumers
run_package_callbacks(uuidkey)
update_explicit_loaded_modules(uuidkey)
else
m = get(loaded_modules, uuidkey, nothing)
if m !== nothing && !haskey(explicit_loaded_modules, uuidkey)
explicit_loaded_modules[uuidkey] = m
run_package_callbacks(uuidkey)
if m !== nothing && !in(uuidkey, explicit_loaded_modules)
update_explicit_loaded_modules(uuidkey)
end
newm = root_module(uuidkey)
end
Expand All @@ -2290,7 +2297,7 @@ const pkgorigins = Dict{PkgId,PkgOrigin}()

const loaded_modules = Dict{PkgId,Module}()
# Emptied on Julia start
const explicit_loaded_modules = Dict{PkgId,Module}()
const explicit_loaded_modules = Set{PkgId}()
const loaded_modules_order = Vector{Module}()
const module_keys = IdDict{Module,PkgId}() # the reverse

Expand All @@ -2313,12 +2320,26 @@ 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
update_explicit_loaded_modules(key)
module_keys[m] = key
end
nothing
end


function update_explicit_loaded_modules(key::PkgId)
if !in(key, explicit_loaded_modules)
push!(explicit_loaded_modules, key)
run_package_callbacks(key)
deps = get(require_map, key, nothing)
if deps !== nothing
for dep in deps
update_explicit_loaded_modules(dep)
end
end
end
end

register_root_module(Core)
register_root_module(Base)
register_root_module(Main)
Expand Down
37 changes: 33 additions & 4 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,7 @@ end
cmd_proj_ext = `$(Base.julia_cmd()) $compile --startup-file=no -e $test_ext_proj`
proj = joinpath(@__DIR__, "project", "Extensions")
cmd_proj_ext = addenv(cmd_proj_ext, "JULIA_LOAD_PATH" => join([joinpath(proj, "HasExtensions.jl"), joinpath(proj, "EnvWithDeps")], sep))
run(cmd_proj_ext)
@test success(cmd_proj_ext)
end

# Sysimage extensions
Expand All @@ -1125,17 +1125,46 @@ end
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")
in(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")
in(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)
@test success(cmd)

# The test below requires that LinearAlgebra is in the sysimage and that it has not been loaded yet
# and that it depends on Libdl.
# 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
proj_libdlext = joinpath(@__DIR__, "project", "Extensions", "GotLibdlExt")
sysimg_ext_test_code = """
uuid_key_la = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra")
Base.in_sysimage(uuid_key_la) || error("LinearAlgebra not in sysimage")
uuid_key_libdl = Base.PkgId(Base.UUID("8f399da3-3557-5675-b5ff-fb832c97cbdb"), "Libdl")
Base.in_sysimage(uuid_key_libdl) || error("Libdl not in sysimage")
using GotLibdlExt
using LinearAlgebra
Base.get_extension(GotLibdlExt, :LibdlExt) 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_libdlext, "@stdlib"], sep))
@test success(cmd)

# Failure of loading some extensions when loading a precompiled package with a trigger in the sysimage
sysimg_ext_test_code = """
uuid_key_la = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra")
using GotLibdlExt
using SparseArrays # loads LinearAlgebra
haskey(Base.loaded_modules, uuid_key_la) || error("LinearAlgebra not loaded")
Base.get_extension(GotLibdlExt, :LibdlExt) 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_libdlext, "@stdlib"], sep))
@test_broken success(cmd)

# Extensions in implicit environments
old_load_path = copy(LOAD_PATH)
Expand Down
9 changes: 9 additions & 0 deletions test/project/Extensions/GotLibdlExt/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name = "GotLibdlExt"
uuid = "a549ab1b-9b57-4c18-8a82-af63d7789335"
version = "0.1.0"

[weakdeps]
Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb"

[extensions]
LibdlExt = "Libdl"
5 changes: 5 additions & 0 deletions test/project/Extensions/GotLibdlExt/ext/LibdlExt.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module LibdlExt

using Libdl, GotLibdlExt

end
5 changes: 5 additions & 0 deletions test/project/Extensions/GotLibdlExt/src/GotLibdlExt.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module GotLibdlExt

greet() = print("Hello World!")

end # module GotLibdlExt