Merge pull request #62 from qzhuyan/dev/william/setopt4

refactor: setopt3 -> setopt4, more safe

c_src/quicer_config.h

@@ -56,7 +56,7 @@ bool get_uint64_from_map(ErlNifEnv *env,
                          uint64_t *value);
 
 ERL_NIF_TERM getopt3(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
-ERL_NIF_TERM setopt3(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
+ERL_NIF_TERM setopt4(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
 
 bool create_settings(ErlNifEnv *env,
                      const ERL_NIF_TERM *emap,
@@ -65,4 +65,10 @@ bool create_settings(ErlNifEnv *env,
 bool
 parse_listen_on(ErlNifEnv *env, ERL_NIF_TERM elisten_on, QUIC_ADDR *Address);
 
+ERL_NIF_TERM set_connection_opt(ErlNifEnv *env,
+                                QuicerConnCTX *c_ctx,
+                                ERL_NIF_TERM optname,
+                                ERL_NIF_TERM optval,
+                                ERL_NIF_TERM elevel);
+
 #endif // __QUICER_CONFIG_H_

c_src/quicer_connection.c

@@ -310,6 +310,7 @@ ServerConnectionCallback(HQUIC Connection,
       acc = c_ctx->owner;
       acc_pid = &(acc->Pid);
 
+      // for fast connect:
       if (!(acc && enif_is_process_alive(c_ctx->env, acc_pid)))
         {
           acc = AcceptorDequeue(
@@ -323,6 +324,12 @@ ServerConnectionCallback(HQUIC Connection,
         }
 
       assert(acc);
+
+      if (!acc->fast_conn)
+        {
+          enif_release_resource(c_ctx);
+        }
+
       // A monitor is automatically removed when it triggers or when the
       // resource is deallocated.
       enif_monitor_process(NULL, c_ctx, acc_pid, &c_ctx->owner_mon);
@@ -381,6 +388,12 @@ ServerConnectionCallback(HQUIC Connection,
       TP_CB_3(shutdown_complete,
               Connection,
               Event->SHUTDOWN_COMPLETE.AppCloseInProgress);
+
+      if (!c_ctx->owner->fast_conn
+          && !Event->SHUTDOWN_COMPLETE.HandshakeCompleted)
+        {
+          enif_release_resource(c_ctx);
+        }
       report = enif_make_tuple3(
           env, ATOM_QUIC, ATOM_CLOSED, enif_make_resource(env, c_ctx));
 
@@ -546,6 +559,22 @@ async_connect3(ErlNifEnv *env,
         }
     }
 
+  ERL_NIF_TERM evalue;
+  if (enif_get_map_value(
+          env, eoptions, ATOM_QUIC_PARAM_CONN_LOCAL_ADDRESS, &evalue))
+    {
+      if (!IS_SAME_TERM(ATOM_OK,
+                        set_connection_opt(env,
+                                           c_ctx,
+                                           ATOM_QUIC_PARAM_CONN_LOCAL_ADDRESS,
+                                           evalue,
+                                           ATOM_FALSE)))
+        {
+          destroy_c_ctx(c_ctx);
+          return ERROR_TUPLE_2(ATOM_CONN_OPEN_ERROR);
+        }
+    }
+
   if (QUIC_FAILED(Status = MsQuic->ConnectionStart(c_ctx->Connection,
                                                    c_ctx->Configuration,
                                                    QUIC_ADDRESS_FAMILY_UNSPEC,
@@ -743,9 +772,13 @@ continue_connection_handshake(QuicerConnCTX *c_ctx)
       return QUIC_STATUS_INTERNAL_ERROR;
     }
 
+  // and releases resource in callback
+  enif_keep_resource(c_ctx);
+
   if (QUIC_FAILED(Status = MsQuic->ConnectionSetConfiguration(
                       c_ctx->Connection, c_ctx->l_ctx->Configuration)))
     {
+      enif_release_resource(c_ctx);
       return Status;
     }
 
@@ -756,6 +789,7 @@ continue_connection_handshake(QuicerConnCTX *c_ctx)
                                             sizeof(QUIC_SETTINGS),
                                             &c_ctx->owner->Settings)))
     {
+      enif_release_resource(c_ctx);
       return Status;
     }
   return Status;

c_src/quicer_nif.c

@@ -1000,7 +1000,7 @@ static ErlNifFunc nif_funcs[] = {
   { "async_close_stream", 3, close_stream3, 0},
   { "sockname", 1, sockname1, 0},
   { "getopt", 3, getopt3, 0},
-  { "setopt", 3, setopt3, 0},
+  { "setopt", 4, setopt4, 0},
   { "controlling_process", 2, controlling_process, 0},
   /* for DEBUG */
   { "get_conn_rid", 1, get_conn_rid1, 1},

docs/todo.org

@@ -5,6 +5,7 @@
 * Features List
 Feature todo list, priority descending
 ** add safety check for closed handlers
+check the is_closed flag or delay the close in ctx dealloc callback?
 
 ** Stream send iolist
 

src/quicer.erl

@@ -411,7 +411,7 @@ getopt(Handle, Opt, Optlevel) ->
 setopt(Handle, param_conn_settings, Value) when is_list(Value) ->
   setopt(Handle, param_conn_settings, maps:from_list(Value));
 setopt(Handle, Opt, Value) ->
-  quicer_nif:setopt(Handle, Opt, Value).
+  quicer_nif:setopt(Handle, Opt, Value, false).
 
 -spec get_stream_id(Stream::stream_handler()) ->
         {ok, integer()} | {error, any()}.

src/quicer_nif.erl

@@ -31,7 +31,7 @@
         , async_close_stream/3
         , sockname/1
         , getopt/3
-        , setopt/3
+        , setopt/4
         , controlling_process/2
         ]).
 
@@ -164,11 +164,11 @@ sockname(_Conn) ->
 getopt(_Handle, _Optname, _IsRaw) ->
   erlang:nif_error(nif_library_not_loaded).
 
--spec setopt(handler(), optname(), any()) ->
+-spec setopt(handler(), optname(), any(), optlevel()) ->
         ok |
         {error, badarg | param_error | internal_error | not_enough_mem} |
         {error, atom_reason()}.
-setopt(_Handle, _Opt, _Value) ->
+setopt(_Handle, _Opt, _Value, _Level) ->
   erlang:nif_error(nif_library_not_loaded).
 
 -spec get_conn_rid(connection_handler()) ->

test/quicer_SUITE.erl

@@ -49,6 +49,7 @@
         , tc_conn_basic_slow_start/1
         , tc_conn_double_close/1
         , tc_conn_other_port/1
+        , tc_conn_with_localaddr/1
         , tc_conn_controlling_process/1
 
         , tc_stream_client_init/1
@@ -72,7 +73,11 @@
         , tc_getopt/1
         , tc_getopt_stream_active/1
         , tc_setopt/1
-        , tc_setopt_conn_local_addr/1
+
+        %% @TODO following two tcs are failing due to:
+        %  https://github.com/microsoft/msquic/issues/2033
+        % , tc_setopt_conn_local_addr/1
+        % , tc_setopt_conn_local_addr_in_use/1
         , tc_strm_opt_active_n/1
         , tc_strm_opt_active_once/1
         , tc_strm_opt_active_1/1
@@ -369,6 +374,26 @@ tc_conn_other_port(Config)->
       ct:fail("timeout")
   end.
 
+tc_conn_with_localaddr(Config)->
+  Port = 5568,
+  Owner = self(),
+  {SPid, Ref} = spawn_monitor(fun() -> simple_conn_server(Owner, Config, Port) end),
+
+  {ok, CPort0} = gen_udp:open(0, [{ip, {127, 0, 0, 1}}]),
+  {ok, {{127, 0, 0, 1}, PortX}} = inet:sockname(CPort0),
+  ok = gen_udp:close(CPort0),
+  receive
+    listener_ready ->
+      {ok, Conn} = quicer:connect("127.0.0.1", Port, [{param_conn_local_address, "127.0.0.1:" ++ integer_to_list(PortX)}
+                                                     | default_conn_opts()], 5000),
+      ?assertEqual({ok, {{127,0,0,1}, PortX}}, quicer:sockname(Conn)),
+      ok = quicer:close_connection(Conn),
+      SPid ! done,
+      ok = ensure_server_exit_normal(Ref)
+  after 1000 ->
+      ct:fail("timeout")
+  end.
+
 tc_stream_client_init(Config) ->
   Port = 4568,
   Owner = self(),
@@ -862,6 +887,7 @@ tc_getstat_closed(Config) ->
       {ok, 4} = quicer:send(Stm, <<"ping">>),
       receive {quic, _, _, _,_,_} -> ok end,
       ok = quicer:close_stream(Stm),
+      ok = quicer:close_connection(Conn),
       case quicer:getstat(Conn, [send_cnt, recv_oct, send_pend]) of
         {error,invalid_parameter} -> ok;
         {error,invalid_state} -> ok;
@@ -1045,8 +1071,13 @@ tc_setopt_conn_local_addr(Config) ->
   %% change local addr with a new port 5060
   ?assertEqual(ok, quicer:setopt(Conn, param_conn_local_address, "127.0.0.1:50600")),
   %% sleep is needed to finish migration at protocol level
-  timer:sleep(200),
-  ?assertEqual({ok, {{127,0,0,1}, 50600}}, quicer:sockname(Stm0)),
+  retry_with(fun() ->
+                 timer:sleep(100),
+                 case quicer:sockname(Stm0) of
+                   {ok, {{127,0,0,1}, 50600}} -> true;
+                   {ok, Other} -> {false, Other}
+                 end
+             end, 20, "addr migration failed"),
   {ok, 5} = quicer:send(Stm0, <<"ping2">>),
   receive
     {quic, <<"ping2">>, Stm0, _, _, _} ->
@@ -1061,6 +1092,52 @@ tc_setopt_conn_local_addr(Config) ->
   SPid ! done,
   ensure_server_exit_normal(Ref).
 
+tc_setopt_conn_local_addr_in_use(Config) ->
+  Port = 4578,
+  Owner = self(),
+  {SPid, Ref} = spawn_monitor(fun() -> echo_server(Owner, Config, Port) end),
+  {ok, Conn} = quicer:connect("127.0.0.1", Port, default_conn_opts(), 5000),
+  {ok, Stm0} = quicer:start_stream(Conn, [{active, true}]),
+  {ok, 5} = quicer:send(Stm0, <<"ping1">>),
+  receive
+    {quic, <<"ping1">>, Stm0, _, _, _} ->
+      ok
+  after 1000 ->
+      ct:fail("recv ping1 timeout")
+  end,
+  {ok, OldAddr} = quicer:sockname(Stm0),
+  %% change local addr with a new random port (0)
+  ?assertEqual(ok, quicer:setopt(Conn, param_conn_local_address, "127.0.0.1:0")),
+  %% sleep is needed to finish migration at protocol level
+  timer:sleep(50),
+  {ok, NewAddr} = quicer:sockname(Stm0),
+  ?assertNotEqual(OldAddr, NewAddr),
+  ?assertNotEqual({ok, {{127,0,0,1}, 50600}}, NewAddr),
+  ?assertNotEqual({ok, {{127,0,0,1}, 50600}}, OldAddr),
+
+  %% Occupy 50600
+  {ok, ESocket} = gen_udp:open(50600, [{ip, element(1, NewAddr)}]),
+  %% change local addr with a new port 5060
+  ?assertEqual({error,address_in_use}, quicer:setopt(Conn, param_conn_local_address, "127.0.0.1:50600")),
+
+  %gen_udp:close(ESocket),
+
+  %% sleep is needed to finish migration at protocol level
+  ct:pal("send after migration failed"),
+  {ok, 5} = quicer:send(Stm0, <<"ping2">>),
+  receive
+    {quic, <<"ping2">>, Stm0, _, _, _} ->
+      ok
+  after 1000 ->
+      ct:fail("recv ping2 timeout")
+  end,
+  %% check with server if peer addr is correct.
+  SPid ! {peer_addr, self()},
+  receive {peer_addr, Peer} -> ok end,
+  ?assertEqual({ok, NewAddr}, Peer),
+  SPid ! done,
+  ensure_server_exit_normal(Ref).
+
 tc_app_echo_server(Config) ->
   Port = 8888,
   application:ensure_all_started(quicer),
@@ -1451,6 +1528,17 @@ active_recv(Stream, Len, BinList) ->
       end
 end.
 
+retry_with(_Fun, 0, ErrorInfo) ->
+  ct:fail(ErrorInfo);
+retry_with(Fun, Retry, ErrorInfo) ->
+  case Fun() of
+    true ->
+      ok;
+    false ->
+      retry_with(Fun, Retry - 1, ErrorInfo);
+    {false, NewErrorInfo} ->
+      retry_with(Fun, Retry - 1, NewErrorInfo)
+  end.
 %%%_* Emacs ====================================================================
 %%% Local Variables:
 %%% allout-layout: t