From 9e35dcfcb17521119a1ca25d974a7c6e5a6bedc9 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash@gmail.com>
Date: Wed, 17 Jan 2018 12:43:05 -0500
Subject: [PATCH] loading: precompile files might try _require the same package
 twice

fix #25604
---
 base/loading.jl | 84 ++++++++++++++++++++++++-------------------------
 test/compile.jl | 37 ++++++++++++++++++++++
 2 files changed, 79 insertions(+), 42 deletions(-)

diff --git a/base/loading.jl b/base/loading.jl
index 6cf2f4c347bf0..b84364d3dce12 100644
--- a/base/loading.jl
+++ b/base/loading.jl
@@ -134,6 +134,31 @@ function _include_from_serialized(path::String, depmods::Vector{Any})
     return restored
 end
 
+function _tryrequire_from_serialized(modname::Symbol, uuid::UInt64, modpath::Union{Nothing, String})
+    if root_module_exists(modname)
+        M = root_module(modname)
+        if module_name(M) === modname && module_uuid(M) === uuid
+            return M
+        end
+    else
+        if modpath === nothing
+            modpath = find_package(string(modname))
+            modpath === nothing && return ErrorException("Required dependency $modname not found in current path.")
+        end
+        mod = _require_search_from_serialized(modname, String(modpath))
+        if !isa(mod, Bool)
+            for callback in package_callbacks
+                invokelatest(callback, modname)
+            end
+            for M in mod::Vector{Any}
+                if module_name(M) === modname && module_uuid(M) === uuid
+                    return M
+                end
+            end
+        end
+    end
+    return nothing
+end
 
 function _require_from_serialized(path::String)
     # loads a precompile cache file, ignoring stale_cachfile tests
@@ -151,28 +176,9 @@ function _require_from_serialized(path::String)
     depmods = Vector{Any}(uninitialized, ndeps)
     for i in 1:ndeps
         modname, uuid = depmodnames[i]
-        if root_module_exists(modname)
-            M = root_module(modname)
-            if module_name(M) === modname && module_uuid(M) === uuid
-                depmods[i] = M
-            end
-        else
-            modpath = find_package(string(modname))
-            modpath === nothing && return ErrorException("Required dependency $modname not found in current path.")
-            mod = _require_search_from_serialized(modname, String(modpath))
-            if !isa(mod, Bool)
-                for M in mod::Vector{Any}
-                    if module_name(M) === modname && module_uuid(M) === uuid
-                        depmods[i] = M
-                        break
-                    end
-                end
-                for callback in package_callbacks
-                    invokelatest(callback, modname)
-                end
-            end
-        end
-        isassigned(depmods, i) || return ErrorException("Required dependency $modname failed to load from a cache file.")
+        dep = _tryrequire_from_serialized(modname, uuid, nothing)
+        dep === nothing && return ErrorException("Required dependency $modname failed to load from a cache file.")
+        depmods[i] = dep::Module
     end
     # then load the file
     return _include_from_serialized(path, depmods)
@@ -184,33 +190,27 @@ end
 function _require_search_from_serialized(mod::Symbol, sourcepath::String)
     paths = find_all_in_cache_path(String(mod)) # cache files for sourcepath are stored keyed by the `mod` symbol name
     for path_to_try in paths::Vector{String}
-        deps = stale_cachefile(sourcepath, path_to_try)
-        if deps === true
+        staledeps = stale_cachefile(sourcepath, path_to_try)
+        if staledeps === true
             continue
         end
-        # finish loading module graph into deps
-        for i in 1:length(deps)
-            dep = deps[i]
+        # finish loading module graph into staledeps
+        for i in 1:length(staledeps)
+            dep = staledeps[i]
             dep isa Module && continue
             modpath, modname, uuid = dep::Tuple{String, Symbol, UInt64}
-            reqmod = _require_search_from_serialized(modname, modpath)
-            if !isa(reqmod, Bool)
-                for M in reqmod::Vector{Any}
-                    if module_name(M) === modname && module_uuid(M) === uuid
-                        deps[i] = M
-                        break
-                    end
-                end
-                for callback in package_callbacks
-                    invokelatest(callback, modname)
-                end
-            end
-            if !isa(deps[i], Module)
+            dep = _tryrequire_from_serialized(modname, uuid, modpath)
+            if dep === nothing
                 @debug "Required dependency $modname failed to load from cache file for $modpath."
-                continue
+                staledeps = true
+                break
             end
+            staledeps[i] = dep::Module
+        end
+        if staledeps === true
+            continue
         end
-        restored = _include_from_serialized(path_to_try, deps)
+        restored = _include_from_serialized(path_to_try, staledeps)
         if isa(restored, Exception)
             @debug "Deserialization checks failed while attempting to load cache from $path_to_try" exception=restored
         else
diff --git a/test/compile.jl b/test/compile.jl
index ba60be9cf02d1..ff8f7157135e7 100644
--- a/test/compile.jl
+++ b/test/compile.jl
@@ -674,4 +674,41 @@ let
     end
 end
 
+let
+    load_path = mktempdir()
+    load_cache_path = mktempdir()
+    try
+        write(joinpath(load_path, "A25604.jl"),
+            """
+            __precompile__(true)
+            module A25604
+            using B25604
+            using C25604
+            end
+            """)
+        write(joinpath(load_path, "B25604.jl"),
+            """
+            __precompile__()
+            module B25604
+            end
+            """)
+        write(joinpath(load_path, "C25604.jl"),
+            """
+            __precompile__()
+            module C25604
+            using B25604
+            end
+            """)
+
+        pushfirst!(LOAD_PATH, load_path)
+        pushfirst!(Base.LOAD_CACHE_PATH, load_cache_path)
+
+        Base.compilecache(:A25604)
+        @test_nowarn @eval using A25604
+    finally
+        rm(load_path, recursive=true)
+        rm(load_cache_path, recursive=true)
+    end
+end
+
 end # !withenv