RFC: during using, define module before include, to solve a minor data-race on UUID

base/boot.jl

@@ -199,7 +199,7 @@ export
     # method reflection
     applicable, invoke,
     # constants
-    nothing, Main
+    nothing, Main, __precompile__
 
 const getproperty = getfield
 const setproperty! = setfield!
@@ -820,4 +820,6 @@ struct Pair{A, B}
     Pair{Any, Any}(@nospecialize(a::Any), @nospecialize(b::Any)) = new(a, b)
 end
 
+function __precompile__ end
+
 ccall(:jl_set_istopmod, Cvoid, (Any, Bool), Core, true)

base/compiler/utilities.jl

@@ -387,8 +387,6 @@ end
 # options #
 ###########
 
-is_root_module(m::Module) = false
-
 inlining_enabled() = (JLOptions().can_inline == 1)
 function coverage_enabled(m::Module)
     ccall(:jl_generating_output, Cint, ()) == 0 || return false # don't alter caches

base/loading.jl

@@ -340,7 +340,7 @@ to get the file name part of the path.
 """
 function pathof(m::Module)
     @lock require_lock begin
-    pkgid = get(module_keys, m, nothing)
+    pkgid = PkgId(m)
     pkgid === nothing && return nothing
     origin = get(pkgorigins, pkgid, nothing)
     origin === nothing && return nothing
@@ -757,6 +757,7 @@ end
 # or an Exception that describes why it couldn't be loaded
 # and it reconnects the Base.Docs.META
 function _include_from_serialized(path::String, depmods::Vector{Any})
+    assert_havelock(require_lock)
     sv = ccall(:jl_restore_incremental, Any, (Cstring, Any), path, depmods)
     if isa(sv, Exception)
         return sv
@@ -950,7 +951,7 @@ Specify whether the file calling this function is precompilable, defaulting to `
 If a module or file is *not* safely precompilable, it should call `__precompile__(false)` in
 order to throw an error if Julia attempts to precompile it.
 """
-@noinline function __precompile__(isprecompilable::Bool=true)
+@noinline function Core.__precompile__(isprecompilable::Bool=true)
     if !isprecompilable && ccall(:jl_generating_output, Cint, ()) != 0
         throw(PrecompilableError())
     end
@@ -1071,37 +1072,29 @@ function require(uuidkey::PkgId)
 end
 
 const loaded_modules = Dict{PkgId,Module}()
-const module_keys = IdDict{Module,PkgId}() # the reverse
 
-is_root_module(m::Module) = @lock require_lock haskey(module_keys, m)
-root_module_key(m::Module) = @lock require_lock module_keys[m]
+function PkgId(m::Module)
+    name = String(nameof(moduleroot(m)))
+    uuid = UUID(ccall(:jl_module_uuid, NTuple{2, UInt64}, (Any,), m))
+    UInt128(uuid) == 0 ? PkgId(name) : PkgId(uuid, name)
+end
 
-function register_root_module(m::Module)
-    # n.b. This is called from C after creating a new module in `Base.__toplevel__`,
-    # instead of adding them to the binding table there.
-    @lock require_lock begin
-    key = PkgId(m, String(nameof(m)))
+function register_root_module(m::Module, key::PkgId=PkgId(m))
+    assert_havelock(require_lock)
     if haskey(loaded_modules, key)
         oldm = loaded_modules[key]
         if oldm !== m
             @warn "Replacing module `$(key.name)`"
         end
     end
     loaded_modules[key] = m
-    module_keys[m] = key
-    end
     nothing
 end
 
+@lock require_lock begin
 register_root_module(Core)
 register_root_module(Base)
 register_root_module(Main)
-
-# This is used as the current module when loading top-level modules.
-# It has the special behavior that modules evaluated in it get added
-# to the loaded_modules table instead of getting bindings.
-baremodule __toplevel__
-using Base
 end
 
 # get a top-level Module from the given key
@@ -1116,9 +1109,8 @@ loaded_modules_array() = @lock require_lock collect(values(loaded_modules))
 function unreference_module(key::PkgId)
     if haskey(loaded_modules, key)
         m = pop!(loaded_modules, key)
-        # need to ensure all modules are GC rooted; will still be referenced
-        # in module_keys
     end
+    nothing
 end
 
 # Returns `nothing` or the name of the newly-created cachefile
@@ -1192,21 +1184,17 @@ function _require(pkg::PkgId)
 
         # just load the file normally via include
         # for unknown dependencies
+        newm = Module(Symbol(pkg.name), false)
         uuid = pkg.uuid
-        uuid = (uuid === nothing ? (UInt64(0), UInt64(0)) : convert(NTuple{2, UInt64}, uuid))
-        old_uuid = ccall(:jl_module_uuid, NTuple{2, UInt64}, (Any,), __toplevel__)
-        if uuid !== old_uuid
-            ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), __toplevel__, uuid)
+        if uuid !== nothing
+            ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), newm, uuid)
         end
+        register_root_module(newm, pkg)
         unlock(require_lock)
         try
-            include(__toplevel__, path)
-            return
+            eval(newm, :(baremodule $(nameof(newm)); $include($newm, $path); end))
         finally
             lock(require_lock)
-            if uuid !== old_uuid
-                ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), __toplevel__, old_uuid)
-            end
         end
     finally
         toplevel_load[] = last
@@ -1362,20 +1350,26 @@ function include_package_for_output(pkg::PkgId, input::String, depot_path::Vecto
     Base._track_dependencies[] = true
     get!(Base.PkgOrigin, Base.pkgorigins, pkg).path = input
     append!(empty!(Base._concrete_dependencies), concrete_deps)
-    uuid_tuple = pkg.uuid === nothing ? (UInt64(0), UInt64(0)) : convert(NTuple{2, UInt64}, pkg.uuid)
-
-    ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), Base.__toplevel__, uuid_tuple)
     if source !== nothing
         task_local_storage()[:SOURCE_PATH] = source
     end
 
+    newm = Module(Symbol(pkg.name), false)
+    uuid = pkg.uuid
+    if uuid !== nothing
+        ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), newm, uuid)
+        # HACK for preferences
+        ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), Main, uuid)
+    end
+    @lock require_lock register_root_module(newm, pkg)
     try
-        Base.include(Base.__toplevel__, input)
+        eval(newm, :(baremodule $(nameof(newm)); $include($newm, $input); end))
     catch ex
         precompilableerror(ex) || rethrow()
         @debug "Aborting `create_expr_cache'" exception=(ErrorException("Declaration of __precompile__(false) not allowed"), catch_backtrace())
         exit(125) # we define status = 125 means PrecompileableError
     end
+    nothing
 end
 
 const PRECOMPILE_TRACE_COMPILE = Ref{String}()

base/pkgid.jl

@@ -9,11 +9,6 @@ struct PkgId
 end
 PkgId(name::AbstractString) = PkgId(nothing, name)
 
-function PkgId(m::Module, name::String = String(nameof(moduleroot(m))))
-    uuid = UUID(ccall(:jl_module_uuid, NTuple{2, UInt64}, (Any,), m))
-    UInt128(uuid) == 0 ? PkgId(name) : PkgId(uuid, name)
-end
-
 ==(a::PkgId, b::PkgId) = a.uuid == b.uuid && a.name == b.name
 
 function hash(pkg::PkgId, h::UInt)

base/reflection.jl

@@ -33,6 +33,8 @@ Base
 """
 parentmodule(m::Module) = ccall(:jl_module_parent, Ref{Module}, (Any,), m)
 
+is_root_module(m::Module) = m === Base || parentmodule(m) === m
+
 """
     moduleroot(m::Module) -> Module
 

src/dump.c

@@ -322,7 +322,7 @@ static void jl_serialize_module(jl_serializer_state *s, jl_module_t *m)
             int j = 0;
             for (i = 0; i < jl_array_len(s->loaded_modules_array); i++) {
                 jl_module_t *mi = (jl_module_t*)jl_array_ptr_ref(s->loaded_modules_array, i);
-                if (!module_in_worklist(mi)) {
+                if (mi != jl_main_module && !module_in_worklist(mi)) {
                     if (m == mi) {
                         write_int32(s->s, j);
                         return;
@@ -1089,7 +1089,7 @@ static void write_mod_list(ios_t *s, jl_array_t *a)
     for (i = 0; i < len; i++) {
         jl_module_t *m = (jl_module_t*)jl_array_ptr_ref(a, i);
         assert(jl_is_module(m));
-        if (!module_in_worklist(m)) {
+        if (m != jl_main_module && !module_in_worklist(m)) {
             const char *modname = jl_symbol_name(m->name);
             size_t l = strlen(modname);
             write_int32(s, l);
@@ -1151,7 +1151,7 @@ static void write_module_path(ios_t *s, jl_module_t *depmod) JL_NOTSAFEPOINT
 
 // serialize the global _require_dependencies array of pathnames that
 // are include dependencies
-static int64_t write_dependency_list(ios_t *s, jl_array_t **udepsp, jl_array_t *mod_array)
+static int64_t write_dependency_list(ios_t *s, jl_array_t **udepsp)
 {
     int64_t initial_pos = 0;
     int64_t pos = 0;
@@ -1211,20 +1211,19 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t **udepsp, jl_array_t *
         JL_GC_PUSH1(&prefs_list);
         if (jl_base_module) {
             // Toplevel module is the module we're currently compiling, use it to get our preferences hash
-            jl_value_t * toplevel = (jl_value_t*)jl_get_global(jl_base_module, jl_symbol("__toplevel__"));
             jl_value_t * prefs_hash_func = jl_get_global(jl_base_module, jl_symbol("get_preferences_hash"));
             jl_value_t * get_compiletime_prefs_func = jl_get_global(jl_base_module, jl_symbol("get_compiletime_preferences"));
 
-            if (toplevel && prefs_hash_func && get_compiletime_prefs_func) {
+            if (prefs_hash_func && get_compiletime_prefs_func) {
                 // Temporary invoke in newest world age
                 size_t last_age = ct->world_age;
                 ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
 
-                // call get_compiletime_prefs(__toplevel__)
-                jl_value_t *args[3] = {get_compiletime_prefs_func, (jl_value_t*)toplevel, NULL};
+                // call prefs_list = get_compiletime_prefs(Main)
+                jl_value_t *args[3] = {get_compiletime_prefs_func, (jl_value_t*)jl_main_module, NULL};
                 prefs_list = (jl_value_t*)jl_apply(args, 2);
 
-                // Call get_preferences_hash(__toplevel__, prefs_list)
+                // Call get_preferences_hash(prefs_list)
                 args[0] = prefs_hash_func;
                 args[2] = prefs_list;
                 prefs_hash = (jl_value_t*)jl_apply(args, 3);
@@ -2262,7 +2261,7 @@ JL_DLLEXPORT int jl_save_incremental(const char *fname, jl_array_t *worklist)
     // write description on contents
     write_work_list(&f);
     // write binary blob from caller
-    int64_t srctextpos = write_dependency_list(&f, &udeps, mod_array);
+    int64_t srctextpos = write_dependency_list(&f, &udeps);
     // write description of requirements for loading
     // this can return errors during deserialize,
     // best to keep it early (before any actual initialization)
@@ -2291,7 +2290,7 @@ JL_DLLEXPORT int jl_save_incremental(const char *fname, jl_array_t *worklist)
     for (i = 0; i < len; i++) {
         jl_module_t *m = (jl_module_t*)jl_array_ptr_ref(mod_array, i);
         assert(jl_is_module(m));
-        if (m->parent == m) // some toplevel modules (really just Base) aren't actually
+        if (m->parent == m) // some toplevel modules (really just Base) aren't actually toplevel
             jl_collect_lambdas_from_mod(lambdas, m);
     }
     jl_collect_methtable_from_mod(lambdas, jl_type_type_mt);

src/module.c

@@ -11,7 +11,7 @@
 extern "C" {
 #endif
 
-JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, uint8_t default_names)
+jl_module_t *jl_new_module_(jl_sym_t *name, uint8_t default_names)
 {
     jl_task_t *ct = jl_current_task;
     const jl_uuid_t uuid_zero = {0, 0};
@@ -58,20 +58,6 @@ uint32_t jl_module_next_counter(jl_module_t *m)
     return jl_atomic_fetch_add(&m->counter, 1);
 }
 
-JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports, uint8_t default_names)
-{
-    // TODO: should we prohibit this during incremental compilation?
-    jl_module_t *m = jl_new_module_(name, default_names);
-    JL_GC_PUSH1(&m);
-    m->parent = jl_main_module; // TODO: this is a lie
-    jl_gc_wb(m, m->parent);
-    if (std_imports)
-        jl_add_standard_imports(m);
-    JL_GC_POP();
-    // TODO: should we somehow try to gc-root this correctly?
-    return (jl_value_t*)m;
-}
-
 JL_DLLEXPORT void jl_set_module_nospecialize(jl_module_t *self, int on)
 {
     self->nospecialize = (on ? -1 : 0);