erts: Remove deprecated nif 'reload' feature

and instead let erlang:load_nif/2 return {error, {reload, _}}
before even trying to load the library
if a NIF library has already been successfully loaded
for the calling module instance.

7 files changed, 80 insertions, 153 deletions

diff --git a/erts/doc/src/erl_nif.xml b/erts/doc/src/erl_nif.xml
index b5dc9037c4..5d55cc9121 100644
--- a/erts/doc/src/erl_nif.xml
+++ b/erts/doc/src/erl_nif.xml
@@ -296,8 +296,8 @@ return term;</code>
           synchronization. This includes terms in process-independent
           environments that are shared between threads. Resource objects also
           require synchronization if you treat them as mutable.</p>
-        <p>The library initialization callbacks <c>load</c>, <c>reload</c>, and
-          <c>upgrade</c> are all thread-safe even for shared state data.</p>
+        <p>The library initialization callbacks <c>load</c> and
+          <c>upgrade</c> are thread-safe even for shared state data.</p>
       </item>
       <tag><marker id="version_management"/>Version Management</tag>
       <item>
@@ -498,7 +498,7 @@ return term;</code>
     <title>Initialization</title>
     <taglist>
       <tag><marker id="ERL_NIF_INIT"/><c>ERL_NIF_INIT(MODULE,
-        ErlNifFunc funcs[], load, reload, upgrade, unload)</c></tag>
+        ErlNifFunc funcs[], load, NULL, upgrade, unload)</c></tag>
       <item>
         <p>This is the magic macro to initialize a NIF library. It
           is to be evaluated in global file scope.</p>
@@ -507,11 +507,14 @@ return term;</code>
           the macro.</p>
         <p><c>funcs</c> is a static array of function descriptors for
           all the implemented NIFs in this library.</p>
-        <p><c>load</c>, <c>reload</c>, <c>upgrade</c> and <c>unload</c>
-          are pointers to functions. One of <c>load</c>, <c>reload</c>, or
+        <p><c>load</c>, <c>upgrade</c> and <c>unload</c>
+          are pointers to functions. One of <c>load</c> or
           <c>upgrade</c> is called to initialize the library.
           <c>unload</c> is called to release the library. All are
           described individually below.</p>
+       <p>The fourth argument <c>NULL</c> is ignored. It
+          was earlier used for the deprectated <c>reload</c> callback
+          which is no longer supported since OTP 20.</p>
         <p>If compiling a NIF for static inclusion through
           <c>--enable-static-nifs</c>, you must define <c>STATIC_ERLANG_NIF</c>
            before the <c>ERL_NIF_INIT</c> declaration.</p>
@@ -539,7 +542,7 @@ return term;</code>
           and there is old code of this module with a loaded NIF library.</p>
         <p>Works as <c>load</c>, except that <c>*old_priv_data</c> already
           contains the value set by the last call to <c>load</c> or
-          <c>reload</c> for the old module code. <c>*priv_data</c> is
+          <c>upgrade</c> for the old module code. <c>*priv_data</c> is
           initialized to <c>NULL</c> when <c>upgrade</c> is called. It is
           allowed to write to both <c>*priv_data</c> and
           <c>*old_priv_data.</c></p> 
@@ -551,27 +554,7 @@ return term;</code>
       <item>
         <p><c>unload</c> is called when the module code that
           the NIF library belongs to is purged as old. New code of the same
-          module may or may not exist. Notice that <c>unload</c> is not
-          called for a replaced library as a consequence of <c>reload</c>.</p>
-      </item>
-      <tag><marker id="reload"/><c>int (*reload)(ErlNifEnv* env, void**
-        priv_data, ERL_NIF_TERM load_info)</c></tag>
-      <item>
-        <note>
-          <p><em>The reload mechanism is deprecated.</em> It was only intended
-            as a development feature. Do not use it as an upgrade method for
-            live production systems. It can be removed in future releases.
-            Ensure to pass <c>reload</c> as <c>NULL</c> to
-            <seealso marker="#ERL_NIF_INIT"><c>ERL_NIF_INIT</c></seealso>
-            to disable it when not used.</p>
-        </note>
-        <p><c>reload</c> is called when the NIF library is loaded and a
-          previously loaded library already exists for this module code.</p>
-        <p>Works as <c>load</c>, except that
-          <c>*priv_data</c> already contains the value set by the
-          previous call to <c>load</c> or <c>reload</c>.</p>
-        <p>The library fails to load if <c>reload</c> returns
-          anything other than <c>0</c> or if <c>reload</c> is <c>NULL</c>.</p> 
+          module may or may not exist.</p>
       </item>
     </taglist>
   </section>
@@ -2249,9 +2232,8 @@ enif_map_iterator_destroy(env, &amp;iter);</code>
           returns <c>NULL</c> and sets <c>*tried</c> to <c>flags</c>.
           It is allowed to set <c>tried</c> to <c>NULL</c>.</p>
         <p>Notice that <c>enif_open_resource_type</c> is only allowed to be
-          called in the three callbacks
-          <seealso marker="#load"><c>load</c></seealso>,
-          <seealso marker="#reload"><c>reload</c></seealso>, and
+          called in the two callbacks
+          <seealso marker="#load"><c>load</c></seealso> and
           <seealso marker="#upgrade"><c>upgrade</c></seealso>.</p>
       </desc>
     </func>
@@ -2305,10 +2287,8 @@ enif_map_iterator_destroy(env, &amp;iter);</code>
       <fsummary>Get the private data of a NIF library.</fsummary>
       <desc>
         <p>Returns the pointer to the private data that was set by
-          <seealso marker="#load"><c>load</c></seealso>,
-          <seealso marker="#reload"><c>reload</c></seealso>, or
+          <seealso marker="#load"><c>load</c></seealso> or
           <seealso marker="#upgrade"><c>upgrade</c></seealso>.</p>
-        <p>Was previously named <c>enif_get_data</c>.</p>
       </desc>
     </func>
 
diff --git a/erts/doc/src/erlang.xml b/erts/doc/src/erlang.xml
index f42c39db5f..d22d40ba8a 100644
--- a/erts/doc/src/erlang.xml
+++ b/erts/doc/src/erlang.xml
@@ -2598,9 +2598,13 @@ os_prompt%</pre>
           <item>The library did not fulfill the requirements as a NIF
             library of the calling module.
           </item>
-          <tag><c>load | reload | upgrade</c></tag>
+          <tag><c>load | upgrade</c></tag>
           <item>The corresponding library callback was unsuccessful.
           </item>
+          <tag><c>reload</c></tag>
+          <item>A NIF library is already loaded for this module instance.
+	    The previously deprecated <c>reload</c> feature was removed in OTP 20.
+          </item>
           <tag><c>old_code</c></tag>
           <item>The call to <c>load_nif/2</c> was made from the old
             code of a module that has been upgraded; this is not
diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index b53d3d4c19..7442e99cb3 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -3145,7 +3145,6 @@ static ErlNifFunc* next_func(ErlNifEntry* entry, int* incrp, ErlNifFunc* func)
 BIF_RETTYPE load_nif_2(BIF_ALIST_2)
 {
     static const char bad_lib[] = "bad_lib";
-    static const char reload[] = "reload";
     static const char upgrade[] = "upgrade";
     char* lib_name = NULL;
     void* handle = NULL;
@@ -3162,7 +3161,6 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
     Eterm ret = am_ok;
     int veto;
     struct erl_module_nif* lib = NULL;
-    int reload_warning = 0;
     struct erl_module_instance* this_mi;
     struct erl_module_instance* prev_mi;
 
@@ -3222,8 +3220,12 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
 	this_mi = module_p->on_load;
     }
 
-    if (init_func == NULL &&
-	     (err=erts_sys_ddll_open(lib_name, &handle, &errdesc)) != ERL_DE_NO_ERROR) {
+    if (this_mi->nif != NULL) {
+        ret = load_nif_error(BIF_P,"reload","NIF library already loaded"
+                             " (reload disallowed since OTP 20).");
+    }
+    else if (init_func == NULL &&
+             (err=erts_sys_ddll_open(lib_name, &handle, &errdesc)) != ERL_DE_NO_ERROR) {
 	const char slogan[] = "Failed to load NIF library";
 	if (strstr(errdesc.str, lib_name) != NULL) {
 	    ret = load_nif_error(BIF_P, "load_failed", "%s: '%s'", slogan, errdesc.str);
@@ -3312,7 +3314,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
 	goto error;
     }
 
-    /* Call load, reload or upgrade:
+    /* Call load or upgrade:
      */
 
 
@@ -3324,82 +3326,33 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
     ASSERT(opened_rt_list == NULL);
     lib->mod = module_p;
     env.mod_nif = lib;
-    if (this_mi->nif != NULL) { /*************** Reload ******************/
-	/*
-	 * Repeated load_nif calls from same Erlang module instance ("reload")
-	 * is deprecated and was only ment as a development feature not to
-	 * be used in production systems. (See warning below)
-	 */
-	int k, old_incr = 0;
-	ErlNifFunc* old_func;
-        lib->priv_data = this_mi->nif->priv_data;
-
-	ASSERT(this_mi->nif->entry != NULL);
-	if (entry->reload == NULL) {
-	    ret = load_nif_error(BIF_P,reload,"Reload not supported by this NIF library.");
-	    goto error;
-	}
-	/* Check that no NIF is removed */
-	old_func = this_mi->nif->entry->funcs;
-	for (k=0; k < this_mi->nif->entry->num_of_funcs; k++) {
-	    int incr = 0;
-	    ErlNifFunc* f = entry->funcs;
-	    for (i=0; i < entry->num_of_funcs; i++) {
-		if (old_func->arity == f->arity
-		    && sys_strcmp(old_func->name, f->name) == 0) {
-		    break;
-		}
-		f = next_func(entry, &incr, f);
-	    }
-	    if (i == entry->num_of_funcs) {
-		ret = load_nif_error(BIF_P,reload,"Reloaded library missing "
-				     "function %T:%s/%u\r\n", mod_atom,
-				     old_func->name, old_func->arity);
-		goto error;
-	    }
-	    old_func = next_func(this_mi->nif->entry, &old_incr, old_func);
-	}
-	erts_pre_nif(&env, BIF_P, lib, NULL);
-	veto = entry->reload(&env, &lib->priv_data, BIF_ARG_2);
-	erts_post_nif(&env);
-	if (veto) {
-	    ret = load_nif_error(BIF_P, reload, "Library reload-call unsuccessful.");
-	}
-	else {
-	    commit_opened_resource_types(lib);
-	    this_mi->nif->entry = NULL; /* to prevent 'unload' callback */
-	    erts_unload_nif(this_mi->nif);
-	    reload_warning = 1;
-	}
+
+    lib->priv_data = NULL;
+    if (prev_mi->nif != NULL) { /**************** Upgrade ***************/
+        void* prev_old_data = prev_mi->nif->priv_data;
+        if (entry->upgrade == NULL) {
+            ret = load_nif_error(BIF_P, upgrade, "Upgrade not supported by this NIF library.");
+            goto error;
+        }
+        erts_pre_nif(&env, BIF_P, lib, NULL);
+        veto = entry->upgrade(&env, &lib->priv_data, &prev_mi->nif->priv_data, BIF_ARG_2);
+        erts_post_nif(&env);
+        if (veto) {
+            prev_mi->nif->priv_data = prev_old_data;
+            ret = load_nif_error(BIF_P, upgrade, "Library upgrade-call unsuccessful.");
+        }
+        else
+            commit_opened_resource_types(lib);
     }
-    else {
-	lib->priv_data = NULL;
-	if (prev_mi->nif != NULL) { /**************** Upgrade ***************/
-	    void* prev_old_data = prev_mi->nif->priv_data;
-	    if (entry->upgrade == NULL) {
-		ret = load_nif_error(BIF_P, upgrade, "Upgrade not supported by this NIF library.");
-		goto error;
-	    }
-		erts_pre_nif(&env, BIF_P, lib, NULL);
-	    veto = entry->upgrade(&env, &lib->priv_data, &prev_mi->nif->priv_data, BIF_ARG_2);
-	    erts_post_nif(&env);
-	    if (veto) {
-		prev_mi->nif->priv_data = prev_old_data;
-		ret = load_nif_error(BIF_P, upgrade, "Library upgrade-call unsuccessful.");
-	    }
-	    else
-		commit_opened_resource_types(lib);
-	}
-	else if (entry->load != NULL) { /********* Initial load ***********/
-	    erts_pre_nif(&env, BIF_P, lib, NULL);
-	    veto = entry->load(&env, &lib->priv_data, BIF_ARG_2);
-	    erts_post_nif(&env);
-	    if (veto) {
-		ret = load_nif_error(BIF_P, "load", "Library load-call unsuccessful.");
-	    }
-	    else
-		commit_opened_resource_types(lib);
-	}
+    else if (entry->load != NULL) { /********* Initial load ***********/
+        erts_pre_nif(&env, BIF_P, lib, NULL);
+        veto = entry->load(&env, &lib->priv_data, BIF_ARG_2);
+        erts_post_nif(&env);
+        if (veto) {
+            ret = load_nif_error(BIF_P, "load", "Library load-call unsuccessful.");
+        }
+        else
+            commit_opened_resource_types(lib);
     }
     if (ret == am_ok) {
 	/*
@@ -3458,14 +3411,6 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
     erts_release_code_write_permission();
     erts_free(ERTS_ALC_T_TMP, lib_name);
 
-    if (reload_warning) {
-	erts_dsprintf_buf_t* dsbufp = erts_create_logger_dsbuf();
-	erts_dsprintf(dsbufp,
-		      "Repeated calls to erlang:load_nif from module '%T'.\n\n"
-		      "The NIF reload mechanism is deprecated and must not "
-		      "be used in production systems.\n", mod_atom);
-	erts_send_warning_to_logger(BIF_P->group_leader, dsbufp);
-    }
     BIF_RET(ret);
 }
 
diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl
index 9c1694fa8a..701b46665c 100644
--- a/erts/emulator/test/nif_SUITE.erl
+++ b/erts/emulator/test/nif_SUITE.erl
@@ -28,7 +28,7 @@
 
 -export([all/0, suite/0,
 	 init_per_testcase/2, end_per_testcase/2,
-         basic/1, reload/1, upgrade/1, heap_frag/1,
+         basic/1, reload_error/1, upgrade/1, heap_frag/1,
          t_on_load/1,
 	 types/1, many_args/1, binaries/1, get_string/1, get_atom/1,
 	 maps/1,
@@ -68,7 +68,7 @@
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
-    [basic, reload, upgrade, heap_frag, types, many_args,
+    [basic, reload_error, upgrade, heap_frag, types, many_args,
      t_on_load,
      binaries, get_string, get_atom, maps, api_macros, from_array,
      iolist_as_binary, resource, resource_binary,
@@ -112,8 +112,8 @@ basic(Config) when is_list(Config) ->
     true = lists:member(?MODULE, erlang:system_info(taints)),
     ok.
 
-%% Test reload callback in nif lib
-reload(Config) when is_list(Config) ->    
+%% Test old reload feature now always fails
+reload_error(Config) when is_list(Config) ->
     TmpMem = tmpmem(),
     ensure_lib_loaded(Config),
 
@@ -127,20 +127,20 @@ reload(Config) when is_list(Config) ->
     hold_nif_mod_priv_data(nif_mod:get_priv_data_ptr()),
     [{load,1,1,101},{get_priv_data_ptr,1,2,102}] = nif_mod_call_history(),    
         
-    ok = nif_mod:load_nif_lib(Config, 2),
-    2 = nif_mod:lib_version(),
-    [{reload,2,1,201},{lib_version,2,2,202}] = nif_mod_call_history(),    
+    {error, {reload, _}} = nif_mod:load_nif_lib(Config, 2),
+    1 = nif_mod:lib_version(),
+    [{lib_version,1,3,103}] = nif_mod_call_history(),
 
-    ok = nif_mod:load_nif_lib(Config, 1),
+    {error, {reload, _}} = nif_mod:load_nif_lib(Config, 1),
     1 = nif_mod:lib_version(),
-    [{reload,1,1,101},{lib_version,1,2,102}] = nif_mod_call_history(),    
+    [{lib_version,1,4,104}] = nif_mod_call_history(),
 
     true = erlang:delete_module(nif_mod),
     [] = nif_mod_call_history(),    
 
     %%false= check_process_code(Pid, nif_mod),
     true = erlang:purge_module(nif_mod),
-    [{unload,1,3,103}] = nif_mod_call_history(),    
+    [{unload,1,5,105}] = nif_mod_call_history(),
 
     true = lists:member(?MODULE, erlang:system_info(taints)),
     true = lists:member(nif_mod, erlang:system_info(taints)),
@@ -828,7 +828,7 @@ resource_binary_do() ->
 -define(RT_CREATE,1).
 -define(RT_TAKEOVER,2).
 
-%% Test resource takeover by module reload and upgrade
+%% Test resource takeover by module upgrade
 resource_takeover(Config) when is_list(Config) ->    
     TmpMem = tmpmem(),
     ensure_lib_loaded(Config),
@@ -893,6 +893,7 @@ resource_takeover(Config) when is_list(Config) ->
     ok = forget_resource(NGX1),
     ?CHECK([], nif_mod_call_history()), % no dtor
 
+    {module,nif_mod} = erlang:load_module(nif_mod,ModBin),
     ok = nif_mod:load_nif_lib(Config, 2,
                               [{resource_type, 0, ?RT_TAKEOVER, "resource_type_A",resource_dtor_A,
                                 ?RT_TAKEOVER},
@@ -911,7 +912,9 @@ resource_takeover(Config) when is_list(Config) ->
                                {resource_type, 4, ?RT_CREATE, "resource_type_null_goneY",null,
                                 ?RT_CREATE}
                               ]),
-    ?CHECK([{reload,2,1,201}], nif_mod_call_history()),
+    ?CHECK([{upgrade,2,1,201}], nif_mod_call_history()),
+    true = erlang:purge_module(nif_mod),
+    ?CHECK([{unload,1,1,106}], nif_mod_call_history()),
 
     BinA2 = read_resource(0,A2),
     ok = forget_resource(A2),
@@ -1221,11 +1224,19 @@ threading_do(Config) ->
     ok = tester:load_nif_lib(Config, "basic"),   
     ok = tester:run(),
 
+    erlang:load_module(tester,ModBin),
+    erlang:purge_module(tester),
     ok = tester:load_nif_lib(Config, "rwlock"),
     ok = tester:run(),
 
+    erlang:load_module(tester,ModBin),
+    erlang:purge_module(tester),
     ok = tester:load_nif_lib(Config, "tsd"),
-    ok = tester:run().
+    ok = tester:run(),
+
+    erlang:delete_module(tester),
+    erlang:purge_module(tester).
+
 
 %% Test NIF message sending
 send(Config) when is_list(Config) ->    
@@ -1513,13 +1524,13 @@ send3_new_state(State, Blob) ->
 neg(Config) when is_list(Config) ->
     TmpMem = tmpmem(),
     {'EXIT',{badarg,_}} = (catch erlang:load_nif(badarg, 0)),
-    {error,{load_failed,_}} = erlang:load_nif("pink_unicorn", 0),
     
     Data = proplists:get_value(data_dir, Config),
     File = filename:join(Data, "nif_mod"),
     {ok,nif_mod,Bin} = compile:file(File, [binary,return_errors]),
     {module,nif_mod} = erlang:load_module(nif_mod,Bin),
 
+    {error,{load_failed,_}} = nif_mod:load_nif_lib(Config, 0),
     {error,{bad_lib,_}} = nif_mod:load_nif_lib(Config, no_init),    
     verify_tmpmem(TmpMem),
     ok.
diff --git a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
index f2b1ef9d24..99534a9694 100644
--- a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
+++ b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
@@ -184,14 +184,6 @@ static void resource_takeover(ErlNifEnv* env, PrivData* priv)
     msgenv_resource_type = rt;
 }
 
-static int reload(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info)
-{
-    PrivData* priv = (PrivData*) *priv_data;
-    add_call(env, priv, "reload");
-    resource_takeover(env,priv);
-    return 0;
-}
-
 static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, ERL_NIF_TERM load_info)
 {
     PrivData* priv = (PrivData*) *old_priv_data;
@@ -2094,4 +2086,4 @@ static ErlNifFunc nif_funcs[] =
     {"format_term_nif", 2, format_term}
 };
 
-ERL_NIF_INIT(nif_SUITE,nif_funcs,load,reload,upgrade,unload)
+ERL_NIF_INIT(nif_SUITE,nif_funcs,load,NULL,upgrade,unload)
diff --git a/erts/emulator/test/nif_SUITE_data/tester.c b/erts/emulator/test/nif_SUITE_data/tester.c
index 257b116322..9e0074d554 100644
--- a/erts/emulator/test/nif_SUITE_data/tester.c
+++ b/erts/emulator/test/nif_SUITE_data/tester.c
@@ -53,7 +53,7 @@ void testcase_free(void *ptr)
 
 void testcase_run(TestCaseState_t *tcs);
 
-static int reload(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info)
+static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, ERL_NIF_TERM load_info)
 {
     return 0;
 }
@@ -70,5 +70,5 @@ static ErlNifFunc nif_funcs[] =
     {"run", 0, run}
 };
 
-ERL_NIF_INIT(tester,nif_funcs,NULL,reload,NULL,NULL)
+ERL_NIF_INIT(tester,nif_funcs,NULL,NULL,upgrade,NULL)
 
diff --git a/erts/emulator/test/trace_call_time_SUITE_data/trace_nif.c b/erts/emulator/test/trace_call_time_SUITE_data/trace_nif.c
index 33b346aab7..0540c25cb9 100644
--- a/erts/emulator/test/trace_call_time_SUITE_data/trace_nif.c
+++ b/erts/emulator/test/trace_call_time_SUITE_data/trace_nif.c
@@ -6,11 +6,6 @@ static int load(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info)
     return 0;
 }
 
-static int reload(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info)
-{
-    return 0;
-}
-
 static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, ERL_NIF_TERM load_info)
 {
     return 0;
@@ -34,4 +29,4 @@ static ErlNifFunc nif_funcs[] =
     {"nif_dec", 1, nif_dec_1}
 };
 
-ERL_NIF_INIT(trace_call_time_SUITE,nif_funcs,load,reload,upgrade,unload)
+ERL_NIF_INIT(trace_call_time_SUITE,nif_funcs,load,NULL,upgrade,unload)