From 47df84d374142c63ecf09796c595f66816ecf3b4 Mon Sep 17 00:00:00 2001 From: Jan Hruban Date: Mon, 18 Nov 2019 14:32:07 +0100 Subject: [PATCH] Load the NIFs in the on_load callback This allows repetitive starting and stopping of the fast_xml application. Since OTP 20, it is not possible to reload NIFs and doing `erlang:load_nif` for the second time ends up in error. Loading the NIF upon the module load instead on application startup solves this problem. There was one gotcha when running the tests. Cover reloads each module with its own cover-enabled version, which has lead to calling the NIFs `upgrade` callback, which was not implemented. Let's add this callback and just call into the `load` callback from it. Also the `start_test` content is not relevant anymore, so it's replaced by testing the subsequent application starting and stopping. --- c_src/fxml.c | 7 ++++++- c_src/fxml_stream.c | 7 ++++++- src/fast_xml.erl | 9 +-------- src/fxml.erl | 5 +++++ src/fxml_stream.erl | 5 +++++ test/fxml_test.erl | 6 ++++-- 6 files changed, 27 insertions(+), 12 deletions(-) diff --git a/c_src/fxml.c b/c_src/fxml.c index aba8e9a..c41524a 100644 --- a/c_src/fxml.c +++ b/c_src/fxml.c @@ -53,6 +53,11 @@ static int load(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info) return 0; } +static int upgrade(ErlNifEnv* env, void** priv, void** old_priv, ERL_NIF_TERM load_info) +{ + return load(env, priv, load_info); +} + static struct buf *init_buf(ErlNifEnv* env) { struct buf *rbuf = ENIF_ALLOC(sizeof(struct buf)); @@ -299,4 +304,4 @@ static ErlNifFunc nif_funcs[] = {"element_to_header", 1, element_to_header} }; -ERL_NIF_INIT(fxml, nif_funcs, load, NULL, NULL, NULL) +ERL_NIF_INIT(fxml, nif_funcs, load, NULL, upgrade, NULL) diff --git a/c_src/fxml_stream.c b/c_src/fxml_stream.c index d80cd59..fb715ee 100644 --- a/c_src/fxml_stream.c +++ b/c_src/fxml_stream.c @@ -810,6 +810,11 @@ static int load(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info) return 0; } +static int upgrade(ErlNifEnv* env, void** priv, void** old_priv, ERL_NIF_TERM load_info) +{ + return load(env, priv, load_info); +} + static ERL_NIF_TERM make_parse_error(ErlNifEnv *env, XML_Parser parser) { enum XML_Error errcode = XML_GetErrorCode(parser); @@ -1057,4 +1062,4 @@ static ErlNifFunc nif_funcs[] = {"change_callback_pid", 2, change_callback_pid_nif} }; -ERL_NIF_INIT(fxml_stream, nif_funcs, load, NULL, NULL, NULL) +ERL_NIF_INIT(fxml_stream, nif_funcs, load, NULL, upgrade, NULL) diff --git a/src/fast_xml.erl b/src/fast_xml.erl index f98cf25..f91cc72 100644 --- a/src/fast_xml.erl +++ b/src/fast_xml.erl @@ -49,14 +49,7 @@ %% @end %%-------------------------------------------------------------------- start(_StartType, _StartArgs) -> - case {fxml:load_nif(), fxml_stream:load_nif()} of - {ok, ok} -> - fxml_sup:start_link(); - {{error,_} = E1, _} -> - E1; - {_, {error,_} = E2} -> - E2 - end. + fxml_sup:start_link(). %%-------------------------------------------------------------------- %% @private diff --git a/src/fxml.erl b/src/fxml.erl index 2f928a8..6652c0f 100644 --- a/src/fxml.erl +++ b/src/fxml.erl @@ -27,6 +27,8 @@ -compile(no_native). +-on_load(init/0). + -export([element_to_binary/1, element_to_header/1, crypt/1, remove_cdata/1, remove_subtags/3, get_cdata/1, get_tag_cdata/1, @@ -41,6 +43,9 @@ -include("fxml.hrl"). -export_type([xmlel/0]). +init() -> + ok = load_nif(). + %% Replace element_to_binary/1 with NIF load_nif() -> SOPath = p1_nif_utils:get_so_path(?MODULE, [fast_xml], "fxml"), diff --git a/src/fxml_stream.erl b/src/fxml_stream.erl index 97f7974..93bece5 100644 --- a/src/fxml_stream.erl +++ b/src/fxml_stream.erl @@ -27,6 +27,8 @@ -compile(no_native). +-on_load(init/0). + -export([new/1, new/2, new/3, parse/2, close/1, reset/1, change_callback_pid/2, parse_element/1]). @@ -53,6 +55,9 @@ -export_type([xml_stream_state/0, xml_stream_el/0]). +init() -> + ok = load_nif(). + load_nif() -> SOPath = p1_nif_utils:get_so_path(?MODULE, [fast_xml], "fxml_stream"), load_nif(SOPath). diff --git a/test/fxml_test.erl b/test/fxml_test.erl index ef96e7b..57ddb27 100644 --- a/test/fxml_test.erl +++ b/test/fxml_test.erl @@ -39,8 +39,10 @@ close(State) -> ?assertEqual(true, fxml_stream:close(State)). start_test() -> - ?assertEqual(ok, fxml:load_nif(p1_nif_utils:get_so_path(fxml, [], "fxml"))), - ?assertEqual(ok, fxml_stream:load_nif(p1_nif_utils:get_so_path(fxml_stream, [], "fxml_stream"))). + ?assertMatch({ok, _}, application:ensure_all_started(fast_xml)), + ?assertMatch(ok, application:stop(fast_xml)), + ?assertMatch({ok, _}, application:ensure_all_started(fast_xml)), + ?assertMatch(ok, application:stop(fast_xml)). tag_test() -> ?assertEqual(#xmlel{name = <<"root">>},