From 51ef1e0b3b1ca8fadfc0a297a840896ae35a5357 Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Thu, 14 Nov 2024 15:44:15 +0100
Subject: [PATCH] feat: check  max_connections against max_fd and
 processes_limit

---
 src/esockd.erl                |  3 ++-
 src/esockd_connection_sup.erl | 39 +++++++++++++++++++++++++++++++----
 test/esockd_SUITE.erl         | 15 +++++++++++++-
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/src/esockd.erl b/src/esockd.erl
index 52bc0e4..bafb581 100644
--- a/src/esockd.erl
+++ b/src/esockd.erl
@@ -415,13 +415,14 @@ ulimit() ->
 
 find_max_fd([]) ->
     %% Magic!
+    %% According to Erlang/OTP doc, erlang:system_info(check_io)):
     %% Returns a list containing miscellaneous information about the emulators
     %% internal I/O checking. Notice that the content of the returned list can
     %% vary between platforms and over time. It is only guaranteed that a list
     %% is returned.
     1023;
 find_max_fd([CheckIoResult | More]) ->
-    case lists:keyfind(max_fds, CheckIoResult) of
+    case lists:keyfind(max_fds, 1, CheckIoResult) of
         {max_fds, N} when is_integer(N) andalso N > 0 ->
             N;
         _ ->
diff --git a/src/esockd_connection_sup.erl b/src/esockd_connection_sup.erl
index 1eabc31..ef4de1b 100644
--- a/src/esockd_connection_sup.erl
+++ b/src/esockd_connection_sup.erl
@@ -63,7 +63,6 @@
           mfargs :: esockd:mfargs()
          }).
 
--define(DEFAULT_MAX_CONNS, 1024).
 -define(TRANSPORT, esockd_transport).
 -define(ERROR_MSG(Format, Args),
         error_logger:error_msg("[~s] " ++ Format, [?MODULE | Args])).
@@ -151,7 +150,7 @@ call(Sup, Req) ->
 init(Opts) ->
     process_flag(trap_exit, true),
     Shutdown = get_value(shutdown, Opts, brutal_kill),
-    MaxConns = get_value(max_connections, Opts, ?DEFAULT_MAX_CONNS),
+    MaxConns = resolve_max_connections(get_value(max_connections, Opts)),
     RawRules = get_value(access_rules, Opts, [{allow, all}]),
     AccessRules = [esockd_access:compile(Rule) || Rule <- RawRules],
     MFA = get_value(connection_mfargs, Opts),
@@ -209,7 +208,10 @@ handle_call({add_rule, RawRule}, _From, State = #state{access_rules = Rules}) ->
             end
     catch
         error:Reason ->
-            error_logger:error_msg("Bad access rule: ~p, compile errro: ~p", [RawRule, Reason]),
+            logger:log(error, #{msg => "bad_access_rule",
+                                rule => RawRule,
+                                compile_error => Reason
+                               }),
             {reply, {error, bad_access_rule}, State}
     end;
 
@@ -285,7 +287,12 @@ get_state_option(connection_mfargs, #state{mfargs = MFA}) ->
     MFA.
 
 set_state_option({max_connections, MaxConns}, State) ->
-    State#state{max_connections = MaxConns};
+    case resolve_max_connections(MaxConns) of
+        MaxConns ->
+            State#state{max_connections = MaxConns};
+        _ ->
+            {error, bad_max_connections}
+    end;
 set_state_option({shutdown, Shutdown}, State) ->
     State#state{shutdown = Shutdown};
 set_state_option({access_rules, Rules}, State) ->
@@ -455,3 +462,27 @@ log(Level, Error, Reason, Pid, #state{mfargs = MFA}) ->
 get_module({M, _F, _A}) -> M;
 get_module({M, _F}) -> M;
 get_module(M) -> M.
+
+resolve_max_connections(Desired) ->
+    MaxFds = esockd:ulimit(),
+    MaxProcs = erlang:system_info(process_limit),
+    resolve_max_connections(Desired, MaxFds, MaxProcs).
+
+resolve_max_connections(undefined, MaxFds, MaxProcs) ->
+    %% not configured
+    min(MaxFds, MaxProcs);
+resolve_max_connections(Desired, MaxFds, MaxProcs) when is_integer(Desired) ->
+    Res = lists:min([Desired, MaxFds, MaxProcs]),
+    case Res < Desired of
+        true ->
+            logger:log(error,
+                       #{msg => "max_connections_config_ignored",
+                         max_fds => MaxFds,
+                         max_processes => MaxProcs,
+                         desired => Desired
+                        }
+                      );
+        false ->
+            ok
+    end,
+    Res.
diff --git a/test/esockd_SUITE.erl b/test/esockd_SUITE.erl
index d7746c3..5f2b63d 100644
--- a/test/esockd_SUITE.erl
+++ b/test/esockd_SUITE.erl
@@ -222,9 +222,22 @@ t_get_set_max_connections(_) ->
     ?assertEqual(16, esockd:get_max_connections({udp_echo, 7000})),
     ok = esockd:close(udp_echo, 7000).
 
+t_get_set_invalid_max_connections(_) ->
+    MaxFd = esockd:ulimit(),
+    MaxProcs = erlang:system_info(process_limit),
+    Invalid = max(MaxFd, MaxProcs) + 1,
+    {ok, _LSup} = esockd:open(echo, 7000, [{connection_mfargs, echo_server}]),
+    Expected = min(MaxFd, MaxProcs),
+    ?assertEqual(Expected, esockd:get_max_connections({echo, 7000})),
+    esockd:set_max_connections({echo, 7000}, 2),
+    ?assertEqual(2, esockd:get_max_connections({echo, 7000})),
+    esockd:set_max_connections({echo, 7000}, Invalid),
+    ?assertEqual(2, esockd:get_max_connections({echo, 7000})),
+    ok = esockd:close(echo, 7000).
+
 t_get_set_max_conn_rate(_) ->
     LimiterOpt = #{module => esockd_limiter, capacity => 100, interval => 1},
-    {ok, _LSup} = esockd:open(echo, 7000, 
+    {ok, _LSup} = esockd:open(echo, 7000,
                               [{limiter, LimiterOpt}, {connection_mfargs, echo_server}]),
     ?assertEqual({100, 1}, esockd:get_max_conn_rate({echo, 7000})),
     esockd:set_max_conn_rate({echo, 7000}, LimiterOpt#{capacity := 50, interval := 2}),