Ensure compilation does not leave random spurious data on failure

Prior to this patch, the parallel compiler could leave spurious
processes and the module definition could leave stale data
in the elixir_modules table.

The issue has been fixed by emitting the proper exit signals
and by using monitors.

Signed-off-by: José Valim <[email protected]>

Author: José Valim

lib/elixir/lib/kernel/parallel_compiler.ex

@@ -96,7 +96,7 @@ defmodule Kernel.ParallelCompiler do
           else
             :elixir_compiler.file(h, Keyword.get(options, :dest))
           end
-          {:compiled, h}
+          {:shutdown, h}
         catch
           kind, reason ->
             {:failure, kind, reason, System.stacktrace}
@@ -166,7 +166,7 @@ defmodule Kernel.ParallelCompiler do
 
         spawn_compilers(entries, original, output, options, waiting, queued, schedulers, result)
 
-      {:DOWN, _down_ref, :process, down_pid, {:compiled, file}} ->
+      {:DOWN, _down_ref, :process, down_pid, {:shutdown, file}} ->
         if callback = Keyword.get(options, :each_file) do
           callback.(file)
         end
@@ -187,9 +187,15 @@ defmodule Kernel.ParallelCompiler do
   defp handle_failure(ref, reason, entries, waiting, queued) do
     if file = find_failure(ref, queued) do
       print_failure(file, reason)
+
       if all_missing?(entries, waiting, queued) do
         collect_failures(queued, length(queued) - 1)
       end
+
+      Enum.each queued, fn {child, _, _} ->
+        Process.exit(child, :kill)
+      end
+
       exit({:shutdown, 1})
     end
   end
@@ -201,7 +207,7 @@ defmodule Kernel.ParallelCompiler do
     end
   end
 
-  defp print_failure(_file, {:compiled, _}) do
+  defp print_failure(_file, {:shutdown, _}) do
     :ok
   end
 

lib/elixir/src/elixir_code_server.erl

@@ -6,12 +6,13 @@
 
 -define(timeout, 30000).
 -record(elixir_code_server, {
-  compilation_status=[],
   argv=[],
   loaded=[],
   at_exit=[],
-  pool={[],0},
   paths={[],[]},
+  mod_pool={[],0},
+  mod_ets=dict:new(),
+  compilation_status=[],
   compiler_options=[{docs,true},{debug_info,true},{warnings_as_errors,false}],
   erl_compiler_options=nil
 }).
@@ -35,8 +36,19 @@ init(ok) ->
   _ = code:ensure_loaded('Elixir.Macro.Env'),
   _ = code:ensure_loaded('Elixir.Module.LocalsTracker'),
   _ = code:ensure_loaded('Elixir.Kernel.LexicalTracker'),
+
+  %% The table where we store module definitions
+  _ = ets:new(elixir_modules, [set, protected, named_table, {read_concurrency, true}]),
+
   {ok, #elixir_code_server{}}.
 
+handle_call({defmodule, Pid, Tuple}, _From, Config) ->
+  {Ref, New} = defmodule(Pid, Tuple, Config),
+  {reply, Ref, New};
+
+handle_call({undefmodule, Ref}, _From, Config) ->
+  {reply, ok, undefmodule(Ref, Config)};
+
 handle_call({acquire, Path}, From, Config) ->
   Current = Config#elixir_code_server.loaded,
   case orddict:find(Path, Current) of
@@ -73,11 +85,11 @@ handle_call({compilation_status, CompilerPid}, _From, Config) ->
    Config#elixir_code_server{compilation_status=CompilationStatusListNew}};
 
 handle_call(retrieve_module_name, _From, Config) ->
-  case Config#elixir_code_server.pool of
+  case Config#elixir_code_server.mod_pool of
     {[H|T], Counter} ->
-      {reply, module_tuple(H), Config#elixir_code_server{pool={T,Counter}}};
+      {reply, module_tuple(H), Config#elixir_code_server{mod_pool={T,Counter}}};
     {[], Counter} ->
-      {reply, module_tuple(Counter), Config#elixir_code_server{pool={[],Counter+1}}}
+      {reply, module_tuple(Counter), Config#elixir_code_server{mod_pool={[],Counter+1}}}
   end;
 
 handle_call(erl_compiler_options, _From, Config) ->
@@ -137,16 +149,19 @@ handle_cast({unload_files, Files}, Config) ->
   Unloaded = lists:foldl(fun(File, Acc) -> orddict:erase(File, Acc) end, Current, Files),
   {noreply, Config#elixir_code_server{loaded=Unloaded}};
 
-handle_cast({return_module_name, H}, #elixir_code_server{pool={T,Counter}} = Config) ->
-  {noreply, Config#elixir_code_server{pool={[H|T],Counter}}};
+handle_cast({return_module_name, H}, #elixir_code_server{mod_pool={T,Counter}} = Config) ->
+  {noreply, Config#elixir_code_server{mod_pool={[H|T],Counter}}};
 
 handle_cast({paths, PA, PZ}, #elixir_code_server{} = Config) ->
   {noreply, Config#elixir_code_server{paths={PA,PZ}}};
 
 handle_cast(Request, Config) ->
   {stop, {badcast, Request}, Config}.
 
-handle_info(_Request, Config) ->
+handle_info({'DOWN', Ref, process, _Pid, _Reason}, Config) ->
+  {noreply, undefmodule(Ref, Config)};
+
+handle_info(_Msg, Config) ->
   {noreply, Config}.
 
 terminate(_Reason, _Config) ->
@@ -158,6 +173,21 @@ code_change(_Old, Config, _Extra) ->
 module_tuple(I) ->
   {list_to_atom("elixir_compiler_" ++ integer_to_list(I)), I}.
 
+defmodule(Pid, Tuple, #elixir_code_server{mod_ets=ModEts} = Config) ->
+  ets:insert(elixir_modules, Tuple),
+  Ref = erlang:monitor(process, Pid),
+  Mod = erlang:element(1, Tuple),
+  {Ref, Config#elixir_code_server{mod_ets=dict:store(Ref, Mod, ModEts)}}.
+
+undefmodule(Ref, #elixir_code_server{mod_ets=ModEts} = Config) ->
+  case dict:find(Ref, ModEts) of
+    {ok, Mod} ->
+      ets:delete(elixir_modules, Mod),
+      Config#elixir_code_server{mod_ets=dict:erase(Ref, ModEts)};
+    error ->
+      Config
+  end.
+
 erl_compiler_options() ->
   Key = "ERL_COMPILER_OPTIONS",
   case os:getenv(Key) of

lib/elixir/src/elixir_module.erl

@@ -51,7 +51,7 @@ do_compile(Line, Module, Block, Vars, E) ->
   check_module_availability(Line, File, Module),
 
   Docs = elixir_compiler:get_opt(docs),
-  {Data, Defs, Clas} = build(Line, File, Module, Docs, ?m(E, lexical_tracker)),
+  {Data, Defs, Clas, Ref} = build(Line, File, Module, Docs, ?m(E, lexical_tracker)),
 
   try
     {Result, NE} = eval_form(Line, Module, Data, Block, Vars, E),
@@ -92,7 +92,7 @@ do_compile(Line, Module, Block, Vars, E) ->
     ets:delete(Data),
     ets:delete(Defs),
     ets:delete(Clas),
-    ets:delete(elixir_modules, Module)
+    elixir_code_server:call({undefmodule, Ref})
   end.
 
 %% Hook that builds both attribute and functions and set up common hooks.
@@ -110,7 +110,8 @@ build(Line, File, Module, Docs, Lexical) ->
   Defs = ets:new(Module, [set, public]),
   Clas = ets:new(Module, [bag, public]),
 
-  ets:insert(elixir_modules, {Module, Data, Defs, Clas, Line, File}),
+  Ref = elixir_code_server:call({defmodule, self(),
+                                 {Module, Data, Defs, Clas, Line, File}}),
 
   ets:insert(Data, {before_compile, []}),
   ets:insert(Data, {after_compile, []}),
@@ -132,7 +133,7 @@ build(Line, File, Module, Docs, Lexical) ->
   elixir_locals:setup(Module),
   elixir_def_overridable:setup(Module),
 
-  {Data, Defs, Clas}.
+  {Data, Defs, Clas, Ref}.
 
 %% Receives the module representation and evaluates it.
 

lib/elixir/src/elixir_sup.erl

@@ -28,5 +28,4 @@ init(ok) ->
    }
   ],
 
-  _ = ets:new(elixir_modules, [set, public, named_table, {read_concurrency, true}]),
   {ok, {{one_for_one, 3, 10}, Workers}}.