Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix concurrent module loading return value #54898

Merged
merged 2 commits into from
Jul 7, 2024
Merged

fix concurrent module loading return value #54898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d7d27b
fix concurrent module loading return value
vtjnash Jun 23, 2024
e78a342
fixup! fix concurrent module loading return value
vtjnash Jun 24, 2024
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
48 changes: 25 additions & 23 deletions base/loading.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1996,8 +1996,12 @@ debug_loading_deadlocks::Bool = true # Enable a slightly more expensive, but mor
function start_loading(modkey::PkgId)
# handle recursive calls to require
assert_havelock(require_lock)
loading = get(package_locks, modkey, nothing)
if loading !== nothing
while true
loading = get(package_locks, modkey, nothing)
if loading === nothing
package_locks[modkey] = current_task() => Threads.Condition(require_lock)
return nothing
end
# load already in progress for this module on the task
task, cond = loading
deps = String[modkey.name]
Expand Down Expand Up @@ -2036,10 +2040,9 @@ function start_loading(modkey::PkgId)
end
throw(ConcurrencyViolationError(msg))
end
return wait(cond)
loading = wait(cond)
loading isa Module && return loading
end
package_locks[modkey] = current_task() => Threads.Condition(require_lock)
return
end

function end_loading(modkey::PkgId, @nospecialize loaded)
Expand Down Expand Up @@ -2418,9 +2421,9 @@ function _require(pkg::PkgId, env=nothing)
# attempt to load the module file via the precompile cache locations
if JLOptions().use_compiled_modules != 0
@label load_from_cache
m = _require_search_from_serialized(pkg, path, UInt128(0), true; reasons)
if m isa Module
return m
loaded = _require_search_from_serialized(pkg, path, UInt128(0), true; reasons)
if loaded isa Module
return loaded
end
end

Expand Down Expand Up @@ -2456,31 +2459,30 @@ function _require(pkg::PkgId, env=nothing)
@goto load_from_cache
end
# spawn off a new incremental pre-compile task for recursive `require` calls
cachefile_or_module = maybe_cachefile_lock(pkg, path) do
# double-check now that we have lock
loaded = maybe_cachefile_lock(pkg, path) do
# double-check the search now that we have lock
m = _require_search_from_serialized(pkg, path, UInt128(0), true)
m isa Module && return m
compilecache(pkg, path; reasons)
return compilecache(pkg, path; reasons)
end
cachefile_or_module isa Module && return cachefile_or_module::Module
cachefile = cachefile_or_module
if isnothing(cachefile) # maybe_cachefile_lock returns nothing if it had to wait for another process
loaded isa Module && return loaded
if isnothing(loaded) # maybe_cachefile_lock returns nothing if it had to wait for another process
@goto load_from_cache # the new cachefile will have the newest mtime so will come first in the search
elseif isa(cachefile, Exception)
if precompilableerror(cachefile)
elseif isa(loaded, Exception)
if precompilableerror(loaded)
verbosity = isinteractive() ? CoreLogging.Info : CoreLogging.Debug
@logmsg verbosity "Skipping precompilation due to precompilable error. Importing $(repr("text/plain", pkg))." exception=m
@logmsg verbosity "Skipping precompilation due to precompilable error. Importing $(repr("text/plain", pkg))." exception=loaded
else
@warn "The call to compilecache failed to create a usable precompiled cache file for $(repr("text/plain", pkg))" exception=m
@warn "The call to compilecache failed to create a usable precompiled cache file for $(repr("text/plain", pkg))" exception=loaded
end
# fall-through to loading the file locally if not incremental
else
cachefile, ocachefile = cachefile::Tuple{String, Union{Nothing, String}}
m = _tryrequire_from_serialized(pkg, cachefile, ocachefile)
if !isa(m, Module)
@warn "The call to compilecache failed to create a usable precompiled cache file for $(repr("text/plain", pkg))" exception=m
cachefile, ocachefile = loaded::Tuple{String, Union{Nothing, String}}
loaded = _tryrequire_from_serialized(pkg, cachefile, ocachefile)
if !isa(loaded, Module)
@warn "The call to compilecache failed to create a usable precompiled cache file for $(repr("text/plain", pkg))" exception=loaded
else
return m
return loaded
end
end
if JLOptions().incremental != 0
Expand Down
17 changes: 11 additions & 6 deletions test/loading.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,11 @@ end
empty!(Base.DEPOT_PATH)
append!(Base.DEPOT_PATH, original_depot_path)

module loaded_pkgid1 end
module loaded_pkgid2 end
module loaded_pkgid3 end
module loaded_pkgid4 end

@testset "loading deadlock detector" begin
pkid1 = Base.PkgId("pkgid1")
pkid2 = Base.PkgId("pkgid2")
Expand All @@ -1221,25 +1226,25 @@ append!(Base.DEPOT_PATH, original_depot_path)
t1 = @async begin
@test nothing === @lock Base.require_lock Base.start_loading(pkid2) # @async module pkgid2; using pkgid1; end
notify(e)
@test "loaded_pkgid1" == @lock Base.require_lock Base.start_loading(pkid1)
@lock Base.require_lock Base.end_loading(pkid2, "loaded_pkgid2")
@test loaded_pkgid1 == @lock Base.require_lock Base.start_loading(pkid1)
@lock Base.require_lock Base.end_loading(pkid2, loaded_pkgid2)
end
wait(e)
reset(e)
t2 = @async begin
@test nothing === @lock Base.require_lock Base.start_loading(pkid3) # @async module pkgid3; using pkgid2; end
notify(e)
@test "loaded_pkgid2" == @lock Base.require_lock Base.start_loading(pkid2)
@lock Base.require_lock Base.end_loading(pkid3, "loaded_pkgid3")
@test loaded_pkgid2 == @lock Base.require_lock Base.start_loading(pkid2)
@lock Base.require_lock Base.end_loading(pkid3, loaded_pkgid3)
end
wait(e)
reset(e)
@test_throws(ConcurrencyViolationError("deadlock detected in loading pkgid3 -> pkgid2 -> pkgid1 -> pkgid3 && pkgid4"),
@lock Base.require_lock Base.start_loading(pkid3)).value # try using pkgid3
@test_throws(ConcurrencyViolationError("deadlock detected in loading pkgid4 -> pkgid4 && pkgid1"),
@lock Base.require_lock Base.start_loading(pkid4)).value # try using pkgid4
@lock Base.require_lock Base.end_loading(pkid1, "loaded_pkgid1") # end
@lock Base.require_lock Base.end_loading(pkid4, "loaded_pkgid4") # end
@lock Base.require_lock Base.end_loading(pkid1, loaded_pkgid1) # end
@lock Base.require_lock Base.end_loading(pkid4, loaded_pkgid4) # end
wait(t2)
wait(t1)
end
Expand Down
8 changes: 5 additions & 3 deletions test/threads_exec.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,23 +1122,25 @@ end

# issue #41546, thread-safe package loading
@testset "package loading" begin
ch = Channel{Bool}(threadpoolsize())
ntasks = max(threadpoolsize(), 4)
ch = Channel{Bool}(ntasks)
barrier = Base.Event()
old_act_proj = Base.ACTIVE_PROJECT[]
try
pushfirst!(LOAD_PATH, "@")
Base.ACTIVE_PROJECT[] = joinpath(@__DIR__, "TestPkg")
@sync begin
for _ in 1:threadpoolsize()
for _ in 1:ntasks
Threads.@spawn begin
put!(ch, true)
wait(barrier)
@eval using TestPkg
end
end
for _ in 1:threadpoolsize()
for _ in 1:ntasks
take!(ch)
end
close(ch)
notify(barrier)
end
@test Base.root_module(@__MODULE__, :TestPkg) isa Module
Expand Down