Skip to content

Commit

Permalink
Gracious loading of unloaded but blacklisted mods
Browse files Browse the repository at this point in the history
rebar3's shell allows people to set applications as blacklisted to
prevent them from being reloaded because that can cause crashes.
However, as part of its normal operations, rebar_paths unloads all
modules that are currently not "owned" by at least one process,
considering them safe to do so.

These two behaviours, put together, lead to an odd thing where some
modules are suddenly unloaded and not in path, and that can be
confusing.

This calls for a unification of both features. We could decide to be
pushing the complexity of rebar3's shell into rebar_path so it knows of
blacklists, but this would be a bad idea because rebar_agent already
owns all the damn hack.

So instead this fix adds an optional call within rebar_agent's
blacklisted applications handling that calls `code:ensure_loaded/1` on
their modules. This avoids forcing any code change that would cause a
crash, but reinstates unloaded paths that could be confusing.

Addresses some comments in erlang#2013
  • Loading branch information
ferd committed Jun 2, 2019
1 parent 2a38e6b commit a399fd0
Showing 1 changed file with 25 additions and 5 deletions.
30 changes: 25 additions & 5 deletions src/rebar_agent.erl
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ maybe_show_warning(State) ->
%% that makes sense.
-spec refresh_paths(rebar_state:t()) -> ok.
refresh_paths(RState) ->

RefreshPaths = application:get_env(rebar, refresh_paths, [all_deps, test]),
ToRefresh = parse_refresh_paths(RefreshPaths, RState, []),
%% Modules from apps we can't reload without breaking functionality
Expand Down Expand Up @@ -201,18 +200,39 @@ refresh_path(Path, Blacklist) ->
false ->
refresh_path_do(Path, App);
true ->
?DEBUG("skipping app ~p, stable copy required", [App])
refresh_path_blacklisted(Path)
end
end
end.

refresh_path_do(Path, App) ->
Files = filelib:wildcard(filename:join([Path, "*.beam"])),
Modules = [list_to_atom(filename:basename(F, ".beam"))
|| F <- Files],
Modules = mods_in_path(Path),
?DEBUG("reloading ~p from ~ts", [Modules, Path]),
code:replace_path(App, Path),
reload_modules(Modules).

%% @private blacklisted paths are not reloaded, but if they were not loaded
%% already, we try and ensure they are loaded once. This is a soft operation
%% that does not provoke crashes in existing processes, but hides issues
%% as seen in issue #2013 comments where some loaded modules that are currently
%% run by no processes get unloaded by rebar_paths, without being loaded back in.
refresh_path_blacklisted(Path) ->
Modules = [M || M <- mods_in_path(Path), not is_loaded(M)],
?DEBUG("ensure ~p loaded", [Modules]),
code:add_pathz(Path), % in case the module is only in a new non-clashing path
_ = [code:ensure_loaded(M) || M <- Modules],
ok.

%% @private fetch module names from a given directory that contains
%% pre-build beam files.
mods_in_path(Path) ->
Files = filelib:wildcard(filename:join([Path, "*.beam"])),
[list_to_atom(filename:basename(F, ".beam")) || F <- Files].

%% @private check that a module is already loaded
is_loaded(Mod) ->
code:is_loaded(Mod) =/= false.

%% @private parse refresh_paths option
%% no_deps means only project_apps's ebin path
%% no_test means no test path
Expand Down

0 comments on commit a399fd0

Please sign in to comment.