From 1eb9ce9b188de9954d3ddc007e79d3625fd23f95 Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 11 Jan 2024 12:45:56 +0100 Subject: [PATCH] Fix listener stop and reload behavior --- c_src/quicer_listener.c | 9 ++++ src/quicer.erl | 2 +- src/quicer_listener.erl | 36 ++++++++++++--- src/quicer_nif.erl | 2 +- test/quicer_listener_SUITE.erl | 83 ++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 9 deletions(-) diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index fa1c858d..402cac7b 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -511,8 +511,17 @@ stop_listener1(ErlNifEnv *env, ret = ERROR_TUPLE_2(ATOM_CLOSED); return ret; // follow otp behaviour? } + enif_mutex_lock(l_ctx->lock); + if (l_ctx->is_stopped) + { + ret = ERROR_TUPLE_2(ATOM_LISTENER_STOPPED); + goto exit; + } l_ctx->is_stopped = TRUE; MsQuic->ListenerStop(l_ctx->Listener); + +exit: + enif_mutex_unlock(l_ctx->lock); put_listener_handle(l_ctx); return ret; } diff --git a/src/quicer.erl b/src/quicer.erl index bf268460..9efdfc41 100644 --- a/src/quicer.erl +++ b/src/quicer.erl @@ -267,7 +267,7 @@ reg_close() -> %% @doc Start a stopped listener with listener handle with new Options. -spec start_listener(listener_handle(), listen_on(), listen_opts()) -> - {ok, pid()} | {error, any()}. + ok | {error, any()}. start_listener(Listener, Port, Options) when is_list(Options) -> start_listener(Listener, Port, maps:from_list(Options)); start_listener(Listener, Port, Options) -> diff --git a/src/quicer_listener.erl b/src/quicer_listener.erl index d68d3412..0c74f600 100644 --- a/src/quicer_listener.erl +++ b/src/quicer_listener.erl @@ -25,6 +25,7 @@ lock/2, unlock/2, reload/2, + reload/3, get_handle/2 ]). @@ -94,6 +95,14 @@ unlock(Pid, Timeout) -> reload(Pid, NewConf) -> gen_server:call(Pid, {reload, NewConf}, infinity). +%% @doc Reload the listener with new *listener* opts and new listen_on. +%% @NOTE: the acceptor opts and stream opts are not reloaded. +%%% if you want to reload them, you should restart the listener (terminate and spawn). +%% @end +-spec reload(pid(), quicer:listen_on(), NewConf :: map()) -> ok | {error, _}. +reload(Pid, ListenOn, NewConf) -> + gen_server:call(Pid, {reload, ListenOn, NewConf}, infinity). + -spec get_handle(pid(), timeout()) -> quicer:listener_handle(). get_handle(Pid, Timeout) -> gen_server:call(Pid, get_handle, Timeout). @@ -159,13 +168,11 @@ handle_call(unlock, _From, State) -> ), {reply, Res, State}; handle_call({reload, NewConf}, _From, State) -> - _ = quicer:stop_listener(State#state.listener), - Res = quicer:start_listener( - State#state.listener, - State#state.listen_on, - NewConf - ), - {reply, Res, State}; + {Res, NewState} = do_reload(State#state.listen_on, NewConf, State), + {reply, Res, NewState}; +handle_call({reload, NewListenOn, NewConf}, _From, State) -> + {Res, NewState} = do_reload(NewListenOn, NewConf, State), + {reply, Res, NewState}; handle_call(Request, _From, State) -> Reply = {error, {unimpl, Request}}, {reply, Reply, State}. @@ -217,3 +224,18 @@ terminate(_Reason, #state{listener = L}) -> %% nif listener has no owner process so we need to close it explicitly. _ = quicer:close_listener(L), ok. + +-spec do_reload(quicer:listen_on(), map(), #state{}) -> {ok | {error, any()}, #state{}}. +do_reload(ListenOn, NewConf, State) -> + _ = quicer:stop_listener(State#state.listener), + Res = quicer:start_listener( + State#state.listener, + ListenOn, + NewConf + ), + case Res of + ok -> + {ok, State#state{listen_on = ListenOn, opts = NewConf}}; + Error -> + {Error, State} + end. diff --git a/src/quicer_nif.erl b/src/quicer_nif.erl index 90357980..d2811356 100644 --- a/src/quicer_nif.erl +++ b/src/quicer_nif.erl @@ -209,7 +209,7 @@ start_listener(_Listener, _ListenOn, _Opts) -> close_listener(_Listener) -> erlang:nif_error(nif_library_not_loaded). --spec stop_listener(listener_handle()) -> ok | {error, closed | badarg}. +-spec stop_listener(listener_handle()) -> ok | {error, closed | listener_stopped | badarg}. stop_listener(_Listener) -> erlang:nif_error(nif_library_not_loaded). diff --git a/test/quicer_listener_SUITE.erl b/test/quicer_listener_SUITE.erl index 9543b30d..6a0d71be 100644 --- a/test/quicer_listener_SUITE.erl +++ b/test/quicer_listener_SUITE.erl @@ -369,6 +369,7 @@ tc_stop_start_listener(Config) -> LConf = default_listen_opts(Config), {ok, L} = quicer:listen(Port, LConf), ok = quicer:stop_listener(L), + ?assertEqual({error, listener_stopped}, quicer:stop_listener(L)), ok = snabbkaffe:retry(100, 10, fun() -> ok = quicer:start_listener(L, Port, LConf) end), ok = quicer:close_listener(L). @@ -554,6 +555,88 @@ tc_listener_conf_reload(Config) -> gen_server:stop(ClientConnPid), quicer_listener:stop_listener(QuicApp). +tc_listener_conf_reload_listen_on(Config) -> + process_flag(trap_exit, true), + DataDir = ?config(data_dir, Config), + ServerConnCallback = example_server_connection, + ServerStreamCallback = example_server_stream, + Port = select_port(), + application:ensure_all_started(quicer), + ListenerOpts = [ + {conn_acceptors, 32}, + {peer_bidi_stream_count, 0}, + {peer_unidi_stream_count, 1} + | default_listen_opts(Config) + ], + ConnectionOpts = [ + {conn_callback, ServerConnCallback}, + {stream_acceptors, 2} + | default_conn_opts() + ], + StreamOpts = [ + {stream_callback, ServerStreamCallback} + | default_stream_opts() + ], + Options = {ListenerOpts, ConnectionOpts, StreamOpts}, + + %% Given a QUIC connection between example client and example server + {ok, QuicApp} = quicer:spawn_listener(sample, Port, Options), + ClientConnOpts = default_conn_opts_verify(Config, ca), + {ok, ClientConnPid} = example_client_connection:start_link( + "localhost", + Port, + {ClientConnOpts, default_stream_opts()} + ), + + ct:pal("C1 status : ~p", [sys:get_status(ClientConnPid)]), + {ok, LHandle} = quicer_listener:get_handle(QuicApp, 5000), + + %% WHEN: the listener is reloaded with ListenOn (new bind address) + NewPort = select_port(), + ok = quicer_listener:reload(QuicApp, NewPort, ListenerOpts), + %% THEN: the listener handle is unchanged + ?assertEqual({ok, LHandle}, quicer_listener:get_handle(QuicApp, 5000)), + + %% THEN: start new connection to old port + ?assertMatch( + {error, transport_down, #{error := _, status := _}}, + quicer:connect( + "localhost", + Port, + default_conn_opts_verify(Config, 'ca'), + 1000 + ) + ), + %% WHEN: start new connection to new port + {ok, Conn2} = quicer:connect( + "localhost", + NewPort, + default_conn_opts_verify(Config, 'ca'), + 5000 + ), + + %% THEN: the new connection shall be established and traffic can be sent and received + {ok, Stream2} = quicer:start_stream( + Conn2, + #{open_flag => ?QUIC_STREAM_OPEN_FLAG_UNIDIRECTIONAL} + ), + {ok, _} = quicer:send(Stream2, <<"ping_from_conn_2">>), + + receive + {quic, new_stream, Stream2Remote, #{is_orphan := true}} -> + quicer:setopt(Stream2Remote, active, true), + ok + end, + + receive + {quic, <<"ping_from_conn_2">>, Stream2Remote, _} -> ok + after 2000 -> + ct:fail("nothing from conn 2"), + quicer_test_lib:report_unhandled_messages() + end, + gen_server:stop(ClientConnPid), + quicer_listener:stop_listener(QuicApp). + tc_stop_close_listener(Config) -> Port = select_port(), {ok, L} = quicer:listen(Port, default_listen_opts(Config)),