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);