Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support setting shell hot code loading blacklists #2013

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Feb 4, 2019

by default, all apps except internal rebar3 ones can be blacklisted.
Some months ago, someone added support for configurable lists within
rebar3 itself using the .app env.

This PR re-exports that functionality from the rebar.config file, so
that one can set something like:

{shell, [
  {app_reload_blacklist, [cowboy, ranch]}
]}.

Which will allow to prevent applications that often crash when being
reloaded from doing so. For example, cowboy and ranch processes can be
stuck in an accept call for multiple reloads in dev, which ends up
causing large failures.

by default, all apps except internal rebar3 ones can be blacklisted.
Some months ago, someone added support for configurable lists within
rebar3 itself using the .app env.

This PR re-exports that functionality from the rebar.config file, so
that one can set something like:

    {shell, [
      {app_reload_blacklist, [cowboy, ranch]}
    ]}.

Which will allow to prevent applications that often crash when being
reloaded from doing so. For example, cowboy and ranch processes can be
stuck in an accept call for multiple reloads in dev, which ends up
causing large failures.
@ferd ferd force-pushed the config-supported-path-blacklist branch from fb984f4 to e3c3c0b Compare February 4, 2019 21:16
@ferd ferd merged commit 5f281b9 into erlang:master Feb 4, 2019
@seriyps
Copy link

seriyps commented Mar 4, 2019

It looks like I have a problem with this option:

in my rebar.config

{shell, [{app_reload_blacklist, [ranch]}]}.

and no special configs for test profile.

then I do

$ rebar3 as test shell
> ranch:info().
% gives me a list of listeners
> PathsBefore = ordsets:from_list(code:get_path()).
> r3:compile().
% successfully compiles, ranch not crashing
> ranch:info().
** exception error: undefined function ranch:info/0
> m(ranch).
** exception error: undefined function ranch:module_info/0
     in function  c:m/1 (c.erl, line 722)
> m(ranch_sup).
** exception error: undefined function ranch_sup:module_info/0
     in function  c:m/1 (c.erl, line 722)
> PathsAfter = ordsets:from_list(code:get_path()).
> ordsets:subtract(PathsBefore, PathsAfter).
["<..>/_build/default/lib/ranch/ebin",
 "<..>/_build/default/plugins/rebar3_proper/ebin"]

If I remove {shell, ...} from rebar.config, ranch crashes on r3:compile(), but no path changes happen.

Rebar3 report
 version 3.9.0
 generated at 2019-03-04T22:35:31+00:00
=================
Please submit this along with your issue at https://github.com/erlang/rebar3/issues (and feel free to edit out private information, if any)
-----------------
Task: 
Entered as:
  
-----------------
Operating System: x86_64-unknown-linux-gnu
ERTS: Erlang/OTP 21 [erts-10.0] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe]
Root Directory: /home/seriy/workspace/erl_installations/21.0
Library directory: /home/seriy/workspace/erl_installations/21.0/lib
-----------------
Loaded Applications:
bbmustache: 1.6.0
certifi: 2.3.1
cf: 0.2.2
common_test: 1.16
compiler: 7.2
crypto: 4.3
cth_readable: 1.4.3
dialyzer: 3.3
edoc: 0.9.3
erlware_commons: 1.3.1
eunit: 2.3.6
eunit_formatters: 0.5.0
getopt: 1.0.1
hex_core: 0.4.0
hipe: 3.18
inets: 7.0
kernel: 6.0
providers: 1.7.0
public_key: 1.6
relx: 3.28.0
sasl: 3.2
snmp: 5.2.11
ssl_verify_fun: 1.1.3
stdlib: 3.5
syntax_tools: 2.1.5
tools: 3.0

-----------------
Escript path: /usr/local/bin/rebar3
Providers:
  app_discovery as auto clean compile compile config cover ct cut deps dialyzer do docs edoc escriptize eunit get-deps help info install install_deps key list lock new owner path pkgs proper publish release relup report repos search shell state tar tree unlock update upgrade upgrade upgrade user version xref 

@seriyps
Copy link

seriyps commented Mar 4, 2019

Managed to reproduce with following minimal config:

$ cat rebar.config
{deps, [{ranch, "1.7.0"}
        ]}.

{shell, [{app_reload_blacklist, [ranch]}]}.

$ cat src/myapp.app.src
{application, myapp,
 [{description, "An OTP application"},
  {vsn, "0.1.0"},
  {registered, []},
  {applications,
   [ranch,
    inets,
    kernel,
    stdlib
   ]},
  {modules, []}
 ]}.

$ rebar3 shell
> m(ranch).
% output is ok
> application:load(ranch).  % this is important!
ok
3> r3:compile().           
===> This feature is experimental and may be modified or removed at any time.
Verifying dependencies...
Compiling myapp
ok
4> m(ranch).               
** exception error: undefined function ranch:module_info/0
     in function  c:m/1 (c.erl, line 722)

@ferd
Copy link
Collaborator Author

ferd commented Mar 5, 2019

That's odd, did not have the problem when I tried it with the app booting and all. I guess the issue comes when the module is loaded but not being used, then it gets dropped off the path during compilation, but never re-added properly somehow?

@seriyps
Copy link

seriyps commented Mar 5, 2019

@ferd it can be reproduced if I do application:ensure_all_started(ranch) as well or if I start some ranch listeners. Fun fact is that my listeners continue to run after r3:compile() (don't know if they will work properly tho).
But not reproduced if I don't do application:load(ranch).

@seriyps
Copy link

seriyps commented Mar 5, 2019

Should I create a separate issue?

@ferd
Copy link
Collaborator Author

ferd commented Mar 5, 2019

Yeah you can create an issue. I'm pretty sure the problem is some interplay on path loading that I had avoided modifying when moving to 3.7.0 but it's an easy enough test case to optimize around.

The reason the old processes keep running is that if the code is being run, the module is not unloaded/hard-purged, so any started code will run fine (which is what I tested when I implemented the blacklist), but it won't work well when code was loaded but inactive (unloads and won't come back, which I had not considered and did not test)

@sirn
Copy link

sirn commented Jun 1, 2019

Sorry for digging up an old PR, but @seriyps did you found a workaround for this?

@seriyps
Copy link

seriyps commented Jun 1, 2019

@sirn no, I haven't.

ferd added a commit to ferd/rebar3 that referenced this pull request Jun 2, 2019
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
@ferd
Copy link
Collaborator Author

ferd commented Jun 2, 2019

See #2099 as an attempt to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants