Skip to content

Commit 13e4fd0

Browse files
committed
precompile: fix a myriad of concurrency and cache handling mistakes
* The LOADING_CACHE PR failed to acquire the lock in many places it was new made mandatory. * The stale_cache_key did not include many relevant parameters. * The `isprecompiled` implementation failed to account for the preferred stdlib logic of loading. * The `isprecompiled` cache failed to account for either newly compiled packages or other changes to the file system. * After parallel precompile noticed a package fails, it would retry with serial precompile, which was very pointless. * Semaphore could be <= 0, leading to deadlock.
1 parent 2d2cb8d commit 13e4fd0

File tree

4 files changed

+192
-116
lines changed

4 files changed

+192
-116
lines changed

base/loading.jl

Lines changed: 85 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ struct LoadingCache
260260
identified::Dict{String, Union{Nothing, Tuple{PkgId, String}}}
261261
located::Dict{Tuple{PkgId, Union{String, Nothing}}, Union{Tuple{String, String}, Nothing}}
262262
end
263-
const LOADING_CACHE = Ref{Union{LoadingCache, Nothing}}(nothing)
263+
const LOADING_CACHE = Ref{Union{LoadingCache, Nothing}}(nothing) # n.b.: all access to and through this are protected by require_lock
264264
LoadingCache() = LoadingCache(load_path(), Dict(), Dict(), Dict(), Set(), Dict(), Dict(), Dict())
265265

266266

@@ -302,10 +302,12 @@ end
302302

303303
# Used by Pkg but not used in loading itself
304304
function find_package(arg) # ::Union{Nothing,String}
305+
@lock require_lock begin
305306
pkgenv = identify_package_env(arg)
306307
pkgenv === nothing && return nothing
307308
pkg, env = pkgenv
308309
return locate_package(pkg, env)
310+
end
309311
end
310312

311313
# is there a better/faster ground truth?
@@ -332,6 +334,7 @@ is also returned, except when the identity is not identified.
332334
"""
333335
identify_package_env(where::Module, name::String) = identify_package_env(PkgId(where), name)
334336
function identify_package_env(where::PkgId, name::String)
337+
assert_havelock(require_lock)
335338
cache = LOADING_CACHE[]
336339
if cache !== nothing
337340
pkg_env = get(cache.identified_where, (where, name), missing)
@@ -364,6 +367,7 @@ function identify_package_env(where::PkgId, name::String)
364367
return pkg_env
365368
end
366369
function identify_package_env(name::String)
370+
assert_havelock(require_lock)
367371
cache = LOADING_CACHE[]
368372
if cache !== nothing
369373
pkg_env = get(cache.identified, name, missing)
@@ -426,11 +430,12 @@ julia> using LinearAlgebra
426430
julia> Base.identify_package(LinearAlgebra, "Pkg") # Pkg is not a dependency of LinearAlgebra
427431
```
428432
"""
429-
identify_package(where::Module, name::String) = _nothing_or_first(identify_package_env(where, name))
430-
identify_package(where::PkgId, name::String) = _nothing_or_first(identify_package_env(where, name))
431-
identify_package(name::String) = _nothing_or_first(identify_package_env(name))
433+
identify_package(where::Module, name::String) = @lock require_lock _nothing_or_first(identify_package_env(where, name))
434+
identify_package(where::PkgId, name::String) = @lock require_lock _nothing_or_first(identify_package_env(where, name))
435+
identify_package(name::String) = @lock require_lock _nothing_or_first(identify_package_env(name))
432436

433437
function locate_package_env(pkg::PkgId, stopenv::Union{String, Nothing}=nothing)::Union{Nothing,Tuple{String,String}}
438+
assert_havelock(require_lock)
434439
cache = LOADING_CACHE[]
435440
if cache !== nothing
436441
pathenv = get(cache.located, (pkg, stopenv), missing)
@@ -508,7 +513,7 @@ julia> Base.locate_package(pkg)
508513
```
509514
"""
510515
function locate_package(pkg::PkgId, stopenv::Union{String, Nothing}=nothing)::Union{Nothing,String}
511-
_nothing_or_first(locate_package_env(pkg, stopenv))
516+
@lock require_lock _nothing_or_first(locate_package_env(pkg, stopenv))
512517
end
513518

514519
"""
@@ -1825,51 +1830,60 @@ function show(io::IO, it::ImageTarget)
18251830
end
18261831

18271832
# should sync with the types of arguments of `stale_cachefile`
1828-
const StaleCacheKey = Tuple{PkgId, UInt128, String, String}
1833+
const StaleCacheKey = Tuple{PkgId, UInt128, String, String, Bool, CacheFlags}
18291834

1830-
function compilecache_path(pkg::PkgId;
1835+
function compilecache_freshest_path(pkg::PkgId;
18311836
ignore_loaded::Bool=false,
18321837
stale_cache::Dict{StaleCacheKey,Bool}=Dict{StaleCacheKey, Bool}(),
18331838
cachepath_cache::Dict{PkgId, Vector{String}}=Dict{PkgId, Vector{String}}(),
1834-
cachepaths::Vector{String}=get!(() -> find_all_in_cache_path(pkg), cachepath_cache, pkg),
1839+
cachepaths::Vector{String}=get(() -> find_all_in_cache_path(pkg), cachepath_cache, pkg),
18351840
sourcepath::Union{String,Nothing}=Base.locate_package(pkg),
18361841
flags::CacheFlags=CacheFlags())
1837-
path = nothing
18381842
isnothing(sourcepath) && error("Cannot locate source for $(repr("text/plain", pkg))")
1839-
for path_to_try in cachepaths
1840-
staledeps = stale_cachefile(sourcepath, path_to_try; ignore_loaded, requested_flags=flags)
1841-
if staledeps === true
1842-
continue
1843-
end
1844-
staledeps, _, _ = staledeps::Tuple{Vector{Any}, Union{Nothing, String}, UInt128}
1845-
# finish checking staledeps module graph
1846-
for dep in staledeps
1847-
dep isa Module && continue
1848-
modpath, modkey, modbuild_id = dep::Tuple{String, PkgId, UInt128}
1849-
modpaths = get!(() -> find_all_in_cache_path(modkey), cachepath_cache, modkey)
1850-
for modpath_to_try in modpaths::Vector{String}
1851-
stale_cache_key = (modkey, modbuild_id, modpath, modpath_to_try)::StaleCacheKey
1852-
if get!(() -> stale_cachefile(stale_cache_key...; ignore_loaded, requested_flags=flags) === true,
1853-
stale_cache, stale_cache_key)
1854-
continue
1843+
try_build_ids = UInt128[UInt128(0)]
1844+
if !ignore_loaded
1845+
let loaded = get(loaded_precompiles, pkg, nothing)
1846+
if loaded !== nothing
1847+
for mod in loaded # try these in reverse original load order to see if one is already valid
1848+
pushfirst!(try_build_ids, module_build_id(mod))
18551849
end
1856-
@goto check_next_dep
18571850
end
1858-
@goto check_next_path
1859-
@label check_next_dep
18601851
end
1861-
try
1862-
# update timestamp of precompilation file so that it is the first to be tried by code loading
1863-
touch(path_to_try)
1864-
catch ex
1865-
# file might be read-only and then we fail to update timestamp, which is fine
1866-
ex isa IOError || rethrow()
1852+
end
1853+
for build_id in try_build_ids
1854+
for path_to_try in cachepaths
1855+
staledeps = stale_cachefile(pkg, build_id, sourcepath, path_to_try; ignore_loaded, requested_flags=flags)
1856+
if staledeps === true
1857+
continue
1858+
end
1859+
staledeps, _, _ = staledeps::Tuple{Vector{Any}, Union{Nothing, String}, UInt128}
1860+
# finish checking staledeps module graph
1861+
for dep in staledeps
1862+
dep isa Module && continue
1863+
modpath, modkey, modbuild_id = dep::Tuple{String, PkgId, UInt128}
1864+
modpaths = get(find_all_in_cache_path, cachepath_cache, modkey)
1865+
for modpath_to_try in modpaths::Vector{String}
1866+
stale_cache_key = (modkey, modbuild_id, modpath, modpath_to_try, ignore_loaded, flags)::StaleCacheKey
1867+
if get!(() -> stale_cachefile(modkey, modbuild_id, modpath, modpath_to_try; ignore_loaded, requested_flags=flags) === true,
1868+
stale_cache, stale_cache_key)
1869+
continue
1870+
end
1871+
@goto check_next_dep
1872+
end
1873+
@goto check_next_path
1874+
@label check_next_dep
1875+
end
1876+
try
1877+
# update timestamp of precompilation file so that it is the first to be tried by code loading
1878+
touch(path_to_try)
1879+
catch ex
1880+
# file might be read-only and then we fail to update timestamp, which is fine
1881+
ex isa IOError || rethrow()
1882+
end
1883+
return path_to_try
1884+
@label check_next_path
18671885
end
1868-
path = path_to_try
1869-
break
1870-
@label check_next_path
18711886
end
1872-
return path
18731887
end
18741888

18751889
"""
@@ -1885,14 +1899,8 @@ fresh julia session specify `ignore_loaded=true`.
18851899
!!! compat "Julia 1.10"
18861900
This function requires at least Julia 1.10.
18871901
"""
1888-
function isprecompiled(pkg::PkgId;
1889-
ignore_loaded::Bool=false,
1890-
stale_cache::Dict{StaleCacheKey,Bool}=Dict{StaleCacheKey, Bool}(),
1891-
cachepath_cache::Dict{PkgId, Vector{String}}=Dict{PkgId, Vector{String}}(),
1892-
cachepaths::Vector{String}=get!(() -> find_all_in_cache_path(pkg), cachepath_cache, pkg),
1893-
sourcepath::Union{String,Nothing}=Base.locate_package(pkg),
1894-
flags::CacheFlags=CacheFlags())
1895-
path = compilecache_path(pkg; ignore_loaded, stale_cache, cachepath_cache, cachepaths, sourcepath, flags)
1902+
function isprecompiled(pkg::PkgId; ignore_loaded::Bool=false)
1903+
path = compilecache_freshest_path(pkg; ignore_loaded)
18961904
return !isnothing(path)
18971905
end
18981906

@@ -1906,7 +1914,7 @@ associated cache is relocatable.
19061914
This function requires at least Julia 1.11.
19071915
"""
19081916
function isrelocatable(pkg::PkgId)
1909-
path = compilecache_path(pkg)
1917+
path = compilecache_freshest_path(pkg)
19101918
isnothing(path) && return false
19111919
io = open(path, "r")
19121920
try
@@ -1926,6 +1934,23 @@ function isrelocatable(pkg::PkgId)
19261934
return true
19271935
end
19281936

1937+
function parse_cache_buildid(cachepath::String)
1938+
f = open(cachepath, "r")
1939+
try
1940+
checksum = isvalid_cache_header(f)
1941+
iszero(checksum) && throw(ArgumentError("Incompatible header in cache file $cachefile."))
1942+
flags = read(f, UInt8)
1943+
n = read(f, Int32)
1944+
n == 0 && error("no module defined in $cachefile")
1945+
skip(f, n) # module name
1946+
uuid = UUID((read(f, UInt64), read(f, UInt64))) # pkg UUID
1947+
build_id = (UInt128(checksum) << 64) | read(f, UInt64)
1948+
return build_id, uuid
1949+
finally
1950+
close(f)
1951+
end
1952+
end
1953+
19291954
# search for a precompile cache file to load, after some various checks
19301955
function _tryrequire_from_serialized(modkey::PkgId, build_id::UInt128)
19311956
assert_havelock(require_lock)
@@ -2683,8 +2708,19 @@ function __require_prelocked(pkg::PkgId, env)
26832708
try
26842709
if !generating_output() && !parallel_precompile_attempted && !disable_parallel_precompile && @isdefined(Precompilation)
26852710
parallel_precompile_attempted = true
2686-
Precompilation.precompilepkgs([pkg]; _from_loading=true, ignore_loaded=false)
2687-
return
2711+
precompiled = Precompilation.precompilepkgs([pkg]; _from_loading=true, ignore_loaded=false)
2712+
# prcompiled returns either nothing, indicating it needs serial precompile,
2713+
# or the entry(ies) that it found would be best to load (possibly because it just created it)
2714+
# or an empty set of entries (indicating the precompile should be skipped)
2715+
if precompiled !== nothing
2716+
isempty(precompiled) && return PrecompilableError() # oops, Precompilation forgot to report what this might actually be
2717+
local cachefile = precompiled[1]
2718+
local ocachefile = nothing
2719+
if JLOptions().use_pkgimages == 1
2720+
ocachefile = ocachefile_from_cachefile(cachefile)
2721+
end
2722+
return cachefile, ocachefile
2723+
end
26882724
end
26892725
triggers = get(EXT_PRIMED, pkg, nothing)
26902726
loadable_exts = nothing

0 commit comments

Comments
 (0)