From 449a093a22af91403c36ca45c620d648d9110ab5 Mon Sep 17 00:00:00 2001 From: Tuncer Ayaz Date: Thu, 1 Sep 2011 18:10:41 +0200 Subject: [erts,kernel,stdlib] fix escript/primary archive reloading If the mtime of an escript/primary archive file changes after being added to the code path, correctly reload the archive and update the cache. The existing code didn't consider that it might be a zip archive and failed: =ERROR REPORT==== 3-Aug-2011::09:21:21 === File operation error: bad_central_directory. Target: /escript_archive/module.beam. Function: get_file. Process: code_server. Thanks David Reid and Hakan Mattson. --- erts/preloaded/src/erl_prim_loader.erl | 108 ++++++++++++++------- lib/kernel/src/code.erl | 13 ++- lib/kernel/src/code_server.erl | 4 +- lib/kernel/test/erl_prim_loader_SUITE.erl | 8 +- lib/stdlib/src/escript.erl | 18 +++- .../archive_script/archive_script_main2.erl | 17 +++- 6 files changed, 119 insertions(+), 49 deletions(-) diff --git a/erts/preloaded/src/erl_prim_loader.erl b/erts/preloaded/src/erl_prim_loader.erl index 14a7a2bf20..e14c8e6370 100644 --- a/erts/preloaded/src/erl_prim_loader.erl +++ b/erts/preloaded/src/erl_prim_loader.erl @@ -49,7 +49,7 @@ prim_read_file_info/2, prim_get_cwd/2]). %% Used by escript and code --export([set_primary_archive/3, release_archives/0]). +-export([set_primary_archive/4, release_archives/0]). -include_lib("kernel/include/file.hrl"). @@ -222,15 +222,16 @@ get_cwd(Drive) -> check_file_result(get_cwd, Drive, request({get_cwd,[Drive]})). -spec set_primary_archive(File :: string() | 'undefined', - ArchiveBin :: binary() | 'undefined', - FileInfo :: #file_info{} | 'undefined') + ArchiveBin :: binary() | 'undefined', + FileInfo :: #file_info{} | 'undefined', + ParserFun :: fun()) -> {ok, [string()]} | {error,_}. -set_primary_archive(undefined, undefined, undefined) -> - request({set_primary_archive, undefined, undefined, undefined}); -set_primary_archive(File, ArchiveBin, FileInfo) +set_primary_archive(undefined, undefined, undefined, ParserFun) -> + request({set_primary_archive, undefined, undefined, undefined, ParserFun}); +set_primary_archive(File, ArchiveBin, FileInfo, ParserFun) when is_list(File), is_binary(ArchiveBin), is_record(FileInfo, file_info) -> - request({set_primary_archive, File, ArchiveBin, FileInfo}). + request({set_primary_archive, File, ArchiveBin, FileInfo, ParserFun}). -spec release_archives() -> 'ok' | {'error', _}. @@ -318,8 +319,11 @@ loop(State, Parent, Paths) -> {get_cwd,[_]=Args} -> {Res,State1} = handle_get_cwd(State, Args), {Res,State1,Paths}; - {set_primary_archive,File,Bin,FileInfo} -> - {Res,State1} = handle_set_primary_archive(State, File, Bin, FileInfo), + {set_primary_archive,File,ArchiveBin,FileInfo,ParserFun} -> + {Res,State1} = + handle_set_primary_archive(State, File, + ArchiveBin, FileInfo, + ParserFun), {Res,State1,Paths}; release_archives -> {Res,State1} = handle_release_archives(State), @@ -359,8 +363,8 @@ handle_get_file(State = #state{loader = efile}, Paths, File) -> handle_get_file(State = #state{loader = inet}, Paths, File) -> ?SAFE2(inet_get_file_from_port(State, File, Paths), State). -handle_set_primary_archive(State= #state{loader = efile}, File, Bin, FileInfo) -> - ?SAFE2(efile_set_primary_archive(State, File, Bin, FileInfo), State). +handle_set_primary_archive(State= #state{loader = efile}, File, ArchiveBin, FileInfo, ParserFun) -> + ?SAFE2(efile_set_primary_archive(State, File, ArchiveBin, FileInfo, ParserFun), State). handle_release_archives(State= #state{loader = efile}) -> ?SAFE2(efile_release_archives(State), State). @@ -484,8 +488,10 @@ efile_get_file_from_port3(State, File, [P | Paths]) -> efile_get_file_from_port3(State, _File, []) -> {{error,enoent},State}. -efile_set_primary_archive(#state{prim_state = PS} = State, File, Bin, FileInfo) -> - {Res, PS2} = prim_set_primary_archive(PS, File, Bin, FileInfo), +efile_set_primary_archive(#state{prim_state = PS} = State, File, + ArchiveBin, FileInfo, ParserFun) -> + {Res, PS2} = prim_set_primary_archive(PS, File, ArchiveBin, + FileInfo, ParserFun), {Res,State#state{prim_state = PS2}}. efile_release_archives(#state{prim_state = PS} = State) -> @@ -791,7 +797,7 @@ prim_release_archives(PS) -> prim_do_release_archives(PS, [{ArchiveFile, DictVal} | KeyVals], Acc) -> Res = case DictVal of - {primary, _PrimZip, _FI} -> + {primary, _PrimZip, _FI, _ParserFun} -> ok; % Keep primary archive {Cache, _FI} -> debug(PS, {release, cache, ArchiveFile}), @@ -809,7 +815,7 @@ prim_do_release_archives(PS, [], []) -> prim_do_release_archives(PS, [], Errors) -> {{error, Errors}, PS#prim_state{primary_archive = undefined}}. -prim_set_primary_archive(PS, undefined, undefined, undefined) -> +prim_set_primary_archive(PS, undefined, undefined, undefined, _ParserFun) -> debug(PS, {set_primary_archive, clean}), case PS#prim_state.primary_archive of undefined -> @@ -817,48 +823,38 @@ prim_set_primary_archive(PS, undefined, undefined, undefined) -> debug(PS, {return, Res}), {Res, PS}; ArchiveFile -> - {primary, PrimZip, _FI} = erase(ArchiveFile), + {primary, PrimZip, _FI, _ParserFun2} = erase(ArchiveFile), ok = prim_zip:close(PrimZip), PS2 = PS#prim_state{primary_archive = undefined}, Res = {ok, []}, debug(PS2, {return, Res}), {Res, PS2} end; -prim_set_primary_archive(PS, ArchiveFile, ArchiveBin, #file_info{} = FileInfo) +prim_set_primary_archive(PS, ArchiveFile, ArchiveBin, + #file_info{} = FileInfo, ParserFun) when is_list(ArchiveFile), is_binary(ArchiveBin) -> %% Try the archive file debug(PS, {set_primary_archive, ArchiveFile, byte_size(ArchiveBin)}), {Res3, PS3} = case PS#prim_state.primary_archive of undefined -> - Fun = - fun({Funny, _GI, _GB}, A) -> - case Funny of - ["", "nibe", RevApp] -> % Reverse ebin - %% Collect ebin directories in archive - Ebin = reverse(RevApp) ++ "/ebin", - {true, [Ebin | A]}; - _ -> - {true, A} - end - end, - Ebins0 = [ArchiveFile], - case open_archive({ArchiveFile, ArchiveBin}, FileInfo, Ebins0, Fun) of - {ok, PrimZip, {RevEbins, FI, _}} -> - Ebins = reverse(RevEbins), + case load_prim_archive(ArchiveFile, ArchiveBin, FileInfo) of + {ok, PrimZip, FI, Ebins} -> debug(PS, {set_primary_archive, Ebins}), - put(ArchiveFile, {primary, PrimZip, FI}), - {{ok, Ebins}, PS#prim_state{primary_archive = ArchiveFile}}; + put(ArchiveFile, {primary, PrimZip, FI, ParserFun}), + {{ok, Ebins}, + PS#prim_state{primary_archive = ArchiveFile}}; Error -> debug(PS, {set_primary_archive, Error}), {Error, PS} end; OldArchiveFile -> debug(PS, {set_primary_archive, clean}), - {primary, PrimZip, _FI} = erase(OldArchiveFile), + {primary, PrimZip, _FI, _ParserFun} = erase(OldArchiveFile), ok = prim_zip:close(PrimZip), PS2 = PS#prim_state{primary_archive = undefined}, - prim_set_primary_archive(PS2, ArchiveFile, ArchiveBin, FileInfo) + prim_set_primary_archive(PS2, ArchiveFile, ArchiveBin, + FileInfo, ParserFun) end, debug(PS3, {return, Res3}), {Res3, PS3}. @@ -1011,7 +1007,7 @@ apply_archive(PS, Fun, Acc, Archive) -> %% put(Archive, {Error, FI}), {Error, PS} end; - {primary, PrimZip, FI} -> + {primary, PrimZip, FI, ParserFun} -> case prim_file:read_file_info(Archive) of {ok, FI2} when FI#file_info.mtime =:= FI2#file_info.mtime -> @@ -1022,6 +1018,16 @@ apply_archive(PS, Fun, Acc, Archive) -> debug(PS, {primary, Error}), {Error, PS} end; + {ok, FI2} -> + ok = clear_cache(Archive, {ok, PrimZip}), + case load_prim_archive(Archive, FI2, ParserFun) of + {ok, PrimZip2, FI3, _Ebins} -> + debug(PS, {cache, {update, Archive}}), + put(Archive, {primary, PrimZip2, FI3, ParserFun}); + Error2 -> + debug(PS, {cache, {clear, Error2}}) + end, + apply_archive(PS, Fun, Acc, Archive); Error -> debug(PS, {cache, {clear, Error}}), clear_cache(Archive, {ok, PrimZip}), @@ -1445,3 +1451,31 @@ normalize(Name, Acc) -> [] -> reverse(Acc) end. + +load_prim_archive(ArchiveFile, ArchiveBin, #file_info{}=FileInfo) -> + Fun = fun({Funny, _GI, _GB}, A) -> + case Funny of + ["", "nibe", RevApp] -> % Reverse ebin + %% Collect ebin directories in archive + Ebin = lists:reverse(RevApp, "/ebin"), + {true, [Ebin | A]}; + _ -> + {true, A} + end + end, + Ebins0 = [ArchiveFile], + case open_archive({ArchiveFile, ArchiveBin}, FileInfo, + Ebins0, Fun) of + {ok, PrimZip, {RevEbins, FI, _}} -> + Ebins = reverse(RevEbins), + {ok, PrimZip, FI, Ebins}; + Error -> + Error + end; +load_prim_archive(ArchiveFile, FileInfo, ParserFun) -> + case ParserFun(ArchiveFile) of + {ok, ArchiveBin} -> + load_prim_archive(ArchiveFile, ArchiveBin, FileInfo); + Error -> + Error + end. diff --git a/lib/kernel/src/code.erl b/lib/kernel/src/code.erl index b7fda69ce0..363072951e 100644 --- a/lib/kernel/src/code.erl +++ b/lib/kernel/src/code.erl @@ -63,7 +63,7 @@ which/1, where_is_file/1, where_is_file/2, - set_primary_archive/3, + set_primary_archive/4, clash/0]). -export_type([load_error_rsn/0, load_ret/0]). @@ -107,7 +107,7 @@ %% unstick_mod(Module) -> true %% is_sticky(Module) -> boolean() %% which(Module) -> Filename | loaded_ret_atoms() | non_existing -%% set_primary_archive((FileName, Bin, FileInfo) -> ok | {error, Reason} +%% set_primary_archive((FileName, ArchiveBin, FileInfo, ParserFun) -> ok | {error, Reason} %% clash() -> ok prints out number of clashes %%---------------------------------------------------------------------------- @@ -481,13 +481,16 @@ where_is_file(Path, File) when is_list(Path), is_list(File) -> -spec set_primary_archive(ArchiveFile :: file:filename(), ArchiveBin :: binary(), - FileInfo :: file:file_info()) + FileInfo :: file:file_info(), + ParserFun :: fun()) -> 'ok' | {'error', atom()}. -set_primary_archive(ArchiveFile0, ArchiveBin, #file_info{} = FileInfo) +set_primary_archive(ArchiveFile0, ArchiveBin, #file_info{} = FileInfo, + ParserFun) when is_list(ArchiveFile0), is_binary(ArchiveBin) -> ArchiveFile = filename:absname(ArchiveFile0), - case call({set_primary_archive, ArchiveFile, ArchiveBin, FileInfo}) of + case call({set_primary_archive, ArchiveFile, ArchiveBin, FileInfo, + ParserFun}) of {ok, []} -> ok; {ok, _Mode, Ebins} -> diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl index a2db7c9790..00ad923466 100644 --- a/lib/kernel/src/code_server.erl +++ b/lib/kernel/src/code_server.erl @@ -394,8 +394,8 @@ handle_call(stop,{_From,_Tag}, S) -> handle_call({is_cached,_File}, {_From,_Tag}, S=#state{cache=no_cache}) -> {reply, no, S}; -handle_call({set_primary_archive, File, ArchiveBin, FileInfo}, {_From,_Tag}, S=#state{mode=Mode}) -> - case erl_prim_loader:set_primary_archive(File, ArchiveBin, FileInfo) of +handle_call({set_primary_archive, File, ArchiveBin, FileInfo, ParserFun}, {_From,_Tag}, S=#state{mode=Mode}) -> + case erl_prim_loader:set_primary_archive(File, ArchiveBin, FileInfo, ParserFun) of {ok, Files} -> {reply, {ok, Mode, Files}, S}; {error, _Reason} = Error -> diff --git a/lib/kernel/test/erl_prim_loader_SUITE.erl b/lib/kernel/test/erl_prim_loader_SUITE.erl index 6f4f27d594..72239641e9 100644 --- a/lib/kernel/test/erl_prim_loader_SUITE.erl +++ b/lib/kernel/test/erl_prim_loader_SUITE.erl @@ -426,7 +426,9 @@ primary_archive(Config) when is_list(Config) -> ExpectedEbins = [Archive, DictDir ++ "/ebin", DummyDir ++ "/ebin"], io:format("ExpectedEbins: ~p\n", [ExpectedEbins]), ?line {ok, FileInfo} = prim_file:read_file_info(Archive), - ?line {ok, Ebins} = rpc:call(Node, erl_prim_loader, set_primary_archive, [Archive, ArchiveBin, FileInfo]), + ?line {ok, Ebins} = rpc:call(Node, erl_prim_loader, set_primary_archive, + [Archive, ArchiveBin, FileInfo, + fun escript:parse_file/1]), ?line ExpectedEbins = lists:sort(Ebins), % assert ?line {ok, TopFiles2} = rpc:call(Node, erl_prim_loader, list_dir, [Archive]), @@ -435,7 +437,9 @@ primary_archive(Config) when is_list(Config) -> ?line ok = test_archive(Node, Archive, DictDir, BeamName), %% Cleanup - ?line {ok, []} = rpc:call(Node, erl_prim_loader, set_primary_archive, [undefined, undefined, undefined]), + ?line {ok, []} = rpc:call(Node, erl_prim_loader, set_primary_archive, + [undefined, undefined, undefined, + fun escript:parse_file/1]), ?line stop_node(Node), ?line ok = file:delete(Archive), ok. diff --git a/lib/stdlib/src/escript.erl b/lib/stdlib/src/escript.erl index 27e70ac4d4..498d850df3 100644 --- a/lib/stdlib/src/escript.erl +++ b/lib/stdlib/src/escript.erl @@ -22,7 +22,7 @@ -export([script_name/0, create/2, extract/2]). %% Internal API. --export([start/0, start/1]). +-export([start/0, start/1, parse_file/1]). %%----------------------------------------------------------------------- @@ -346,7 +346,8 @@ parse_and_run(File, Args, Options) -> case Source of archive -> {ok, FileInfo} = file:read_file_info(File), - case code:set_primary_archive(File, FormsOrBin, FileInfo) of + case code:set_primary_archive(File, FormsOrBin, FileInfo, + fun escript:parse_file/1) of ok when CheckOnly -> case code:load_file(Module) of {module, _} -> @@ -396,6 +397,19 @@ parse_and_run(File, Args, Options) -> %% Parse script %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +%% Only used as callback by erl_prim_loader +parse_file(File) -> + try parse_file(File, false) of + {_Source, _Module, FormsOrBin, _HasRecs, _Mode} + when is_binary(FormsOrBin) -> + {ok, FormsOrBin}; + _ -> + {error, no_archive_bin} + catch + throw:Reason -> + {error, Reason} + end. + parse_file(File, CheckOnly) -> {HeaderSz, NextLineNo, Fd, Sections} = parse_header(File, false), diff --git a/lib/stdlib/test/escript_SUITE_data/archive_script/archive_script_main2.erl b/lib/stdlib/test/escript_SUITE_data/archive_script/archive_script_main2.erl index de56579998..431a51b0e5 100644 --- a/lib/stdlib/test/escript_SUITE_data/archive_script/archive_script_main2.erl +++ b/lib/stdlib/test/escript_SUITE_data/archive_script/archive_script_main2.erl @@ -21,6 +21,8 @@ -export([main/1]). +-include_lib("kernel/include/file.hrl"). + -define(DUMMY, archive_script_dummy). -define(DICT, archive_script_dict). @@ -32,7 +34,7 @@ main(MainArgs) -> io:format("dummy:~p\n",[[E || E <- ErlArgs, element(1, E) =:= ?DUMMY]]), %% Start the applications - {error, {not_started, ?DICT}} = application:start(archive_script_dummy), + {error, {not_started, ?DICT}} = application:start(?DUMMY), ok = application:start(?DICT), ok = application:start(?DUMMY), @@ -57,4 +59,17 @@ main(MainArgs) -> ok = ?DICT:erase(Tab, Key), error = ?DICT:find(Tab, Key), ok = ?DICT:erase(Tab), + + %% Check mtime related caching bug with escript/primary archive files + Escript = escript:script_name(), + {ok, FileInfo} = file:read_file_info(Escript), + %% Modify mtime of archive file and try to reload module + FileInfo2 = FileInfo#file_info{mtime=calendar:now_to_local_time(now())}, + ok = file:write_file_info(Escript, FileInfo2), + Module = ?DICT, + {file, _} = code:is_loaded(Module), + true = code:delete(Module), + false = code:is_loaded(Module), + {module, Module} = code:ensure_loaded(Module), + ok. -- cgit v1.2.3 From 7875e98fcac4a45607a99ba08a421206a7cf1586 Mon Sep 17 00:00:00 2001 From: Tuncer Ayaz Date: Wed, 19 Oct 2011 13:27:18 +0200 Subject: escript_SUITE: remove gratuitous space --- lib/stdlib/test/escript_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stdlib/test/escript_SUITE.erl b/lib/stdlib/test/escript_SUITE.erl index 7ed1ee742a..3f2617fd13 100644 --- a/lib/stdlib/test/escript_SUITE.erl +++ b/lib/stdlib/test/escript_SUITE.erl @@ -356,7 +356,7 @@ beam_script(Config) when is_list(Config) -> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Create an archive file containing two entire applications plus two %% alternate main modules. Generate a new escript containing the archive -%% (with .app and .beam files and ) and the escript header. +%% (with .app and .beam files and) and the escript header. archive_script(Config) when is_list(Config) -> %% Copy the orig files to priv_dir -- cgit v1.2.3