diff options
author | Luke Bakken <luke@bakken.io> | 2021-12-31 14:38:29 -0800 |
---|---|---|
committer | Luke Bakken <luke@bakken.io> | 2022-01-03 11:33:36 -0800 |
commit | 7f0285834e9bea689e26cc283412b52ded2397dc (patch) | |
tree | 74f67d7c86457635bba1efe7644f7942619fc498 | |
parent | 0bd8d41b72e32435178520fd382f46e835f312fd (diff) | |
download | rabbitmq-server-git-7f0285834e9bea689e26cc283412b52ded2397dc.tar.gz |
Fix all uses of file:read_file/1lukebakken/fix-all-read-file
This is to address another memory leak on win32 reported here:
https://groups.google.com/g/rabbitmq-users/c/UE-wxXerJl8
"RabbitMQ constant memory increase (binary_alloc) in idle state"
The root cause is the Prometheus plugin making repeated calls to `rabbit_misc:otp_version/0` which then calls `file:read_file/1` and leaks memory on win32.
See https://github.com/erlang/otp/issues/5527 for the report to the Erlang team.
Turn `badmatch` into actual error
10 files changed, 42 insertions, 17 deletions
diff --git a/deps/rabbit/apps/rabbitmq_prelaunch/src/rabbit_prelaunch_conf.erl b/deps/rabbit/apps/rabbitmq_prelaunch/src/rabbit_prelaunch_conf.erl index 0df414bad7..99130b8d37 100644 --- a/deps/rabbit/apps/rabbitmq_prelaunch/src/rabbit_prelaunch_conf.erl +++ b/deps/rabbit/apps/rabbitmq_prelaunch/src/rabbit_prelaunch_conf.erl @@ -553,7 +553,7 @@ get_passphrase(ConfigEntryDecoder) -> io:format(IoDevice, "~n", []), PP; {file, Filename} -> - {ok, File} = file:read_file(Filename), + {ok, File} = rabbit_misc:raw_read_file(Filename), [PP|_] = binary:split(File, [<<"\r\n">>, <<"\n">>]), PP; PP -> diff --git a/deps/rabbit/src/rabbit.erl b/deps/rabbit/src/rabbit.erl index 48ef9ec439..b0d3ea8d29 100644 --- a/deps/rabbit/src/rabbit.erl +++ b/deps/rabbit/src/rabbit.erl @@ -1517,7 +1517,7 @@ motd() -> undefined -> undefined; File -> - case file:read_file(File) of + case rabbit_misc:raw_read_file(File) of {ok, MOTD} -> string:trim(MOTD, trailing, [$\r,$\n]); {error, _} -> undefined end diff --git a/deps/rabbit/src/rabbit_definitions_import_local_filesystem.erl b/deps/rabbit/src/rabbit_definitions_import_local_filesystem.erl index 896fcaa40e..d8e5961128 100644 --- a/deps/rabbit/src/rabbit_definitions_import_local_filesystem.erl +++ b/deps/rabbit/src/rabbit_definitions_import_local_filesystem.erl @@ -36,7 +36,7 @@ load(Proplist) when is_list(Proplist) -> undefined -> {error, "local definition file path is not configured: local_path is not set"}; Path -> rabbit_log:debug("Asked to import definitions from a local file or directory at '~s'", [Path]), - case file:read_file_info(Path) of + case file:read_file_info(Path, [raw]) of {ok, FileInfo} -> %% same check is used by Cuttlefish validation, this is to be extra defensive IsReadable = (element(4, FileInfo) == read) or (element(4, FileInfo) == read_write), @@ -130,7 +130,7 @@ load_from_multiple_files([File|Rest]) -> load_from_single_file(Path) -> rabbit_log:debug("Will try to load definitions from a local file or directory at '~s'", [Path]), - case file:read_file(Path) of + case rabbit_misc:raw_read_file(Path) of {ok, Body} -> rabbit_log:info("Applying definitions from file at '~s'", [Path]), import_raw(Body); diff --git a/deps/rabbit_common/src/rabbit_misc.erl b/deps/rabbit_common/src/rabbit_misc.erl index 112647c581..98eb3b3f7d 100644 --- a/deps/rabbit_common/src/rabbit_misc.erl +++ b/deps/rabbit_common/src/rabbit_misc.erl @@ -13,6 +13,8 @@ -include("rabbit_framing.hrl"). -include("rabbit_misc.hrl"). +-include_lib("kernel/include/file.hrl"). + -ifdef(TEST). -export([decompose_pid/1, compose_pid/4]). -endif. @@ -78,6 +80,7 @@ -export([rpc_call/4, rpc_call/5]). -export([get_gc_info/1]). -export([group_proplists_by/2]). +-export([raw_read_file/1]). %% Horrible macro to use in guards -define(IS_BENIGN_EXIT(R), @@ -1218,7 +1221,7 @@ version() -> otp_release() -> File = filename:join([code:root_dir(), "releases", erlang:system_info(otp_release), "OTP_VERSION"]), - case file:read_file(File) of + case raw_read_file(File) of {ok, VerBin} -> %% 17.0 or later, we need the file for the minor version string:strip(binary_to_list(VerBin), both, $\n); @@ -1399,6 +1402,25 @@ rpc_call(Node, Mod, Fun, Args, Timeout) -> get_gc_info(Pid) -> rabbit_runtime:get_gc_info(Pid). +-spec raw_read_file(Filename) -> {ok, Binary} | {error, Reason} when + Filename :: file:name_all(), + Binary :: binary(), + Reason :: file:posix() | badarg | terminated | system_limit. +raw_read_file(File) -> + try + % Note: this works around the win32 file leak in file:read_file/1 + % https://github.com/erlang/otp/issues/5527 + {ok, FInfo} = file:read_file_info(File, [raw]), + {ok, Fd} = file:open(File, [read, raw, binary]), + try + file:read(Fd, FInfo#file_info.size) + after + file:close(Fd) + end + catch + error:{badmatch, Error} -> Error + end. + %% ------------------------------------------------------------------------- %% Begin copypasta from gen_server2.erl diff --git a/deps/rabbit_common/src/rabbit_semver_parser.erl b/deps/rabbit_common/src/rabbit_semver_parser.erl index 3a036021f7..0456de1ae4 100644 --- a/deps/rabbit_common/src/rabbit_semver_parser.erl +++ b/deps/rabbit_common/src/rabbit_semver_parser.erl @@ -15,9 +15,12 @@ -define(p_zero_or_more,true). - -spec file(file:name()) -> any(). -file(Filename) -> case file:read_file(Filename) of {ok,Bin} -> parse(Bin); Err -> Err end. +file(Filename) -> + case rabbit_misc:raw_read_file(Filename) of + {ok,Bin} -> parse(Bin); + Err -> Err + end. -spec parse(binary() | list()) -> any(). parse(List) when is_list(List) -> parse(unicode:characters_to_binary(List)); diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_certificate_expiration.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_certificate_expiration.erl index 3d6dcd1062..fe0944fb79 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_certificate_expiration.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_certificate_expiration.erl @@ -127,7 +127,7 @@ read_cert(undefined) -> read_cert({pem, Pem}) -> Pem; read_cert(Path) -> - case file:read_file(Path) of + case rabbit_misc:raw_read_file(Path) of {ok, Bin} -> Bin; Err -> diff --git a/deps/rabbitmq_peer_discovery_k8s/src/rabbit_peer_discovery_k8s.erl b/deps/rabbitmq_peer_discovery_k8s/src/rabbit_peer_discovery_k8s.erl index ce02cce6cd..3eb09519ae 100644 --- a/deps/rabbitmq_peer_discovery_k8s/src/rabbit_peer_discovery_k8s.erl +++ b/deps/rabbitmq_peer_discovery_k8s/src/rabbit_peer_discovery_k8s.erl @@ -118,7 +118,7 @@ get_config_key(Key, Map) -> make_request() -> M = ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY), - {ok, Token} = file:read_file(get_config_key(k8s_token_path, M)), + {ok, Token} = rabbit_misc:raw_read_file(get_config_key(k8s_token_path, M)), Token1 = binary:replace(Token, <<"\n">>, <<>>), ?HTTPC_MODULE:get( get_config_key(k8s_scheme, M), @@ -169,7 +169,7 @@ extract_node_list(Response) -> -spec base_path(events | endpoints, term()) -> string(). base_path(Type, Args) -> M = ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY), - {ok, Namespace} = file:read_file(get_config_key(k8s_namespace_path, M)), + {ok, Namespace} = rabbit_misc:raw_read_file(get_config_key(k8s_namespace_path, M)), NameSpace1 = binary:replace(Namespace, <<"\n">>, <<>>), rabbit_peer_discovery_httpc:build_path([api, v1, namespaces, NameSpace1, Type, Args]). @@ -219,9 +219,9 @@ generate_v1_event(Namespace, Name, Type, Reason, Message, Timestamp, HostName) - -spec send_event(term(),term(), term()) -> {ok, term()} | {error, term()}. send_event(Type, Reason, Message) -> M = ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY), - {ok, Token} = file:read_file(get_config_key(k8s_token_path, M)), + {ok, Token} = rabbit_misc:raw_read_file(get_config_key(k8s_token_path, M)), Token1 = binary:replace(Token, <<"\n">>, <<>>), - {ok, NameSpace} = file:read_file( + {ok, NameSpace} = rabbit_misc:raw_read_file( get_config_key(k8s_namespace_path, M)), NameSpace1 = binary:replace(NameSpace, <<"\n">>, <<>>), diff --git a/deps/rabbitmq_tracing/src/rabbit_tracing_files.erl b/deps/rabbitmq_tracing/src/rabbit_tracing_files.erl index fa260a4077..882c802ed1 100644 --- a/deps/rabbitmq_tracing/src/rabbit_tracing_files.erl +++ b/deps/rabbitmq_tracing/src/rabbit_tracing_files.erl @@ -37,7 +37,7 @@ full_path(Name0) -> %%-------------------------------------------------------------------- file_info(Name) -> - Size = case file:read_file_info(full_path(Name)) of + Size = case file:read_file_info(full_path(Name), [raw]) of {ok, Info} -> Info#file_info.size; {error, Error} -> diff --git a/deps/rabbitmq_tracing/src/rabbit_tracing_wm_file.erl b/deps/rabbitmq_tracing/src/rabbit_tracing_wm_file.erl index acea4077b8..ed23c6900d 100644 --- a/deps/rabbitmq_tracing/src/rabbit_tracing_wm_file.erl +++ b/deps/rabbitmq_tracing/src/rabbit_tracing_wm_file.erl @@ -38,7 +38,7 @@ serve(ReqData, Context) -> serve(Name) -> Path = rabbit_tracing_files:full_path(Name), - {ok, Content} = file:read_file(Path), + {ok, Content} = rabbit_misc:raw_read_file(Path), Content. delete_resource(ReqData, Context) -> diff --git a/deps/rabbitmq_trust_store/src/rabbit_trust_store_file_provider.erl b/deps/rabbitmq_trust_store/src/rabbit_trust_store_file_provider.erl index 88cd55a535..0adad0005f 100644 --- a/deps/rabbitmq_trust_store/src/rabbit_trust_store_file_provider.erl +++ b/deps/rabbitmq_trust_store/src/rabbit_trust_store_file_provider.erl @@ -47,7 +47,7 @@ extract_cert(Path, FileName) -> scan_then_parse(Absolute). scan_then_parse(Filename) when is_list(Filename) -> - {ok, Bin} = file:read_file(Filename), + {ok, Bin} = rabbit_misc:raw_read_file(Filename), [{'Certificate', Data, not_encrypted}] = public_key:pem_decode(Bin), Data. @@ -64,11 +64,11 @@ list_certs_0(Path) -> FileNames). modification_time(Path) -> - {ok, Info} = file:read_file_info(Path, [{time, posix}]), + {ok, Info} = file:read_file_info(Path, [raw, {time, posix}]), Info#file_info.mtime. file_content_hash(Path) -> - {ok, Data} = file:read_file(Path), + {ok, Data} = rabbit_misc:raw_read_file(Path), erlang:phash2(Data). directory_path(Config) -> |