diff --git a/base/loading.jl b/base/loading.jl index 1c4eb2235b31d..4d6c190f00d22 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -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] @@ -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) @@ -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 @@ -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 diff --git a/test/loading.jl b/test/loading.jl index 72589d4210adc..41b5cc9596f7d 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -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") @@ -1221,16 +1226,16 @@ 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) @@ -1238,8 +1243,8 @@ append!(Base.DEPOT_PATH, original_depot_path) @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 diff --git a/test/threads_exec.jl b/test/threads_exec.jl index 7f4775cae0055..595f8991d58d7 100644 --- a/test/threads_exec.jl +++ b/test/threads_exec.jl @@ -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