Merge pull request #185 from qzhuyan/test/william/improve-nif-coverage

test: improve NIF coverage

.github/workflows/cover.yaml

@@ -37,8 +37,9 @@ jobs:
  uses: coverallsapp/github-action@master
  with:
  github-token: ${{ secrets.GITHUB_TOKEN }}
- flag-name: run-${{ matrix.test_number }}
+ flag-name: run-c-lcov
  parallel: true
+ github-branch: ${{ github.ref_name }}
 
  - name: Coveralls Erl
  env:

c_src/quicer_connection.c

@@ -201,17 +201,17 @@ _IRQL_requires_max_(DISPATCH_LEVEL)
  BOOLEAN is_destroy = FALSE;
  QUIC_STATUS status = QUIC_STATUS_SUCCESS;
 
- assert(Connection == c_ctx->Connection);
- if (!(Connection == c_ctx->Connection))
+ // Connecion Handle must match unless NULL (closed)
+ assert(Connection == c_ctx->Connection || NULL == c_ctx->Connection);
+
+ if (Connection == NULL)
  {
- c_ctx->Connection = Connection;
+ return status;
  }
 
  enif_mutex_lock(c_ctx->lock);
  TP_CB_3(event, (uintptr_t)Connection, Event->Type);
 
- // dbg("client connection event: %d", Event->Type);
-
  switch (Event->Type)
  {
  case QUIC_CONNECTION_EVENT_CONNECTED:
@@ -256,14 +256,8 @@ _IRQL_requires_max_(DISPATCH_LEVEL)
  // The connection has completed the shutdown process and is ready to be
  // safely cleaned up.
  //
- // This is special case for client,
- // it could happen that the connection is opened but never get started.
  // @see async_connect3
- // in this case, we don't need to report closed to the owner
- if (!c_ctx->is_closed) // owner doesn't know it is closed
- {
- status = handle_connection_event_shutdown_complete(c_ctx, Event);
- }
+ status = handle_connection_event_shutdown_complete(c_ctx, Event);
  is_destroy = TRUE;
  c_ctx->is_closed = TRUE; // client shutdown completed
  break;
@@ -329,11 +323,12 @@ ServerConnectionCallback(HQUIC Connection,
  BOOLEAN is_destroy = FALSE;
  QUIC_STATUS status = QUIC_STATUS_SUCCESS;
 
- assert(Connection == c_ctx->Connection);
+ // Connecion Handle must match unless NULL (closed)
+ assert(Connection == c_ctx->Connection || NULL == c_ctx->Connection);
 
- if (!(Connection == c_ctx->Connection))
+ if (Connection == NULL)
  {
- c_ctx->Connection = Connection;
+ return status;
  }
 
  enif_mutex_lock(c_ctx->lock);
@@ -516,9 +511,7 @@ async_connect3(ErlNifEnv *env,
  }
 
  // allocate config_resource for client connection
- if (NULL
- == (c_ctx->config_resource
- = enif_alloc_resource(ctx_config_t, sizeof(QuicerConfigCTX))))
+ if (NULL == (c_ctx->config_resource = init_config_ctx()))
  {
  res = ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY);
  goto Error;
@@ -536,8 +529,6 @@ async_connect3(ErlNifEnv *env,
  goto Error;
  }
 
- enif_monitor_process(NULL, c_ctx, &c_ctx->owner->Pid, &c_ctx->owner_mon);
-
  ERL_NIF_TERM ecacertfile;
  X509_STORE *trusted = NULL;
  if (enif_get_map_value(env, eoptions, ATOM_CACERTFILE, &ecacertfile))
@@ -596,10 +587,15 @@ async_connect3(ErlNifEnv *env,
  c_ctx,
  &(c_ctx->Connection))))
  {
+ assert(c_ctx->Connection == NULL);
  res = ERROR_TUPLE_2(ATOM_CONN_OPEN_ERROR);
  goto Error;
  }
  }
+
+ assert(c_ctx->is_closed);
+ c_ctx->is_closed = FALSE; // connection opened.
+
  ERL_NIF_TERM essl_keylogfile;
  if (enif_get_map_value(
  env, eoptions, ATOM_SSL_KEYLOGFILE_NAME, &essl_keylogfile))
@@ -698,26 +694,38 @@ async_connect3(ErlNifEnv *env,
  // @TODO client async_connect_3 should able to take a config_resource as
  // input ERL TERM so that we don't need to call ClientLoadConfiguration
  //
+ assert(!c_ctx->is_closed && c_ctx->Connection);
  if (QUIC_FAILED(Status = MsQuic->ConnectionStart(
  c_ctx->Connection,
  c_ctx->config_resource->Configuration,
  QUIC_ADDRESS_FAMILY_UNSPEC,
  host,
  port)))
  {
+ AcceptorDestroy(c_ctx->owner);
+ c_ctx->owner = NULL;
+
+ /* Although MsQuic internally close the connection after failed to start,
+ we still do not need to set is_closed here, we expect callback to set
+ it while handling the shutdown complete event otherwise could cause
+ race cond.
+ */
+ // c_ctx->is_closed = TRUE;
+
+ c_ctx->Connection = NULL;
+
  res = ERROR_TUPLE_2(ATOM_CONN_START_ERROR);
- enif_release_resource(c_ctx->config_resource);
+ TP_NIF_3(start_fail, (uintptr_t)(c_ctx->Connection), Status);
  goto Error;
  }
- c_ctx->is_closed = FALSE; // connection started
+
+ assert(c_ctx->owner);
+ enif_monitor_process(NULL, c_ctx, &c_ctx->owner->Pid, &c_ctx->owner_mon);
  eHandle = enif_make_resource(env, c_ctx);
 
  return SUCCESS(eHandle);
 
 Error:
- // Error exit, it must not be started!
- assert(c_ctx->is_closed);
-
  if (c_ctx->Connection)
  { // when is opened
 
@@ -760,7 +768,10 @@ async_connect3(ErlNifEnv *env,
  MsQuic->ConnectionClose(c_ctx->Connection);
  // prevent double ConnectionClose
  c_ctx->Connection = NULL;
+ c_ctx->is_closed = TRUE;
  }
+ // Error exit, it must be closed or Handle is NULL
+ assert(c_ctx->is_closed || NULL == c_ctx->Connection);
  return res;
 }
 
@@ -1151,6 +1162,7 @@ handle_connection_event_shutdown_complete(
  TP_CB_3(shutdown_complete,
  (uintptr_t)c_ctx->Connection,
  Event->SHUTDOWN_COMPLETE.AppCloseInProgress);
+
  ERL_NIF_TERM props_name[] = { ATOM_IS_HANDSHAKE_COMPLETED,
  ATOM_IS_PEER_ACKED,
  ATOM_IS_APP_CLOSING };
@@ -1165,8 +1177,11 @@ handle_connection_event_shutdown_complete(
  props_name,
  props_value,
  3);
- enif_send(NULL, &(c_ctx->owner->Pid), NULL, report);
 
+ if (c_ctx->owner)
+ {
+ enif_send(NULL, &(c_ctx->owner->Pid), NULL, report);
+ }
  //
  // Now inform the stream acceptors
  //

c_src/quicer_ctx.c

@@ -29,9 +29,7 @@ init_l_ctx()
  }
  CxPlatZeroMemory(l_ctx, sizeof(QuicerListenerCTX));
  l_ctx->env = enif_alloc_env();
- l_ctx->config_resource
- = enif_alloc_resource(ctx_config_t, sizeof(QuicerConfigCTX));
- CxPlatZeroMemory(l_ctx->config_resource, sizeof(QuicerConfigCTX));
+ l_ctx->config_resource = init_config_ctx();
  l_ctx->acceptor_queue = AcceptorQueueNew();
  l_ctx->lock = enif_mutex_create("quicer:l_ctx");
  l_ctx->cacertfile = NULL;
@@ -46,7 +44,7 @@ deinit_l_ctx(QuicerListenerCTX *l_ctx)
  AcceptorQueueDestroy(l_ctx->acceptor_queue);
  if (l_ctx->config_resource)
  {
- enif_release_resource(l_ctx->config_resource);
+ destroy_config_ctx(l_ctx->config_resource);
  }
  enif_mutex_destroy(l_ctx->lock);
  enif_free_env(l_ctx->env);

c_src/quicer_listener.c

@@ -447,6 +447,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[])
  Status = MsQuic->ListenerStart(
  l_ctx->Listener, alpn_buffers, alpn_buffer_length, &Address)))
  {
+ TP_NIF_3(start_fail, (uintptr_t)(l_ctx->Listener), Status);
  destroy_l_ctx(l_ctx);
  return ERROR_TUPLE_3(ATOM_LISTENER_START_ERROR, ATOM_STATUS(Status));
  }
@@ -473,6 +474,7 @@ close_listener1(ErlNifEnv *env,
 
  // It is safe to close it without holding the lock
  // This also ensures no ongoing listener callbacks
+ // This is a blocking call. @TODO have async version or use dirty scheduler
  MsQuic->ListenerClose(l);
 
  return ATOM_OK;

c_src/quicer_nif.c

@@ -753,6 +753,7 @@ resource_conn_dealloc_callback(__unused_parm__ ErlNifEnv *env, void *obj)
  QuicerConnCTX *c_ctx = (QuicerConnCTX *)obj;
  TP_CB_3(start, (uintptr_t)c_ctx->Connection, c_ctx->is_closed);
  // must be closed otherwise will trigger callback and casue race cond.
+ // This ensures no callbacks during cleanup here.
  assert(c_ctx->is_closed == TRUE); // in dealloc
  if (c_ctx->Connection)
  {
@@ -842,6 +843,7 @@ resource_config_dealloc_callback(__unused_parm__ ErlNifEnv *env,
  {
  MsQuic->ConfigurationClose(config_ctx->Configuration);
  }
+ deinit_config_ctx(config_ctx);
  TP_CB_3(end, (uintptr_t)obj, 0);
 }
 

c_src/quicer_stream.c

@@ -73,10 +73,6 @@ ServerStreamCallback(HQUIC Stream, void *Context, QUIC_STREAM_EVENT *Event)
  TP_CB_3(event, (uintptr_t)Stream, Event->Type);
  switch (Event->Type)
  {
- case QUIC_STREAM_EVENT_START_COMPLETE:
- // Only for Local initiated stream
- status = handle_stream_event_start_complete(s_ctx, Event);
- break;
 
  case QUIC_STREAM_EVENT_SEND_COMPLETE:
  //
@@ -110,12 +106,6 @@ ServerStreamCallback(HQUIC Stream, void *Context, QUIC_STREAM_EVENT *Event)
  //
  status = handle_stream_event_peer_receive_aborted(s_ctx, Event);
  break;
- case QUIC_STREAM_EVENT_PEER_ACCEPTED:
- //
- // The peer aborted its send direction of the stream.
- //
- status = handle_stream_event_peer_accepted(s_ctx, Event);
- break;
  case QUIC_STREAM_EVENT_SHUTDOWN_COMPLETE:
  //
  // Both directions of the stream have been shut down and MsQuic is done
@@ -998,6 +988,7 @@ handle_stream_event_recv(HQUIC Stream,
  return status;
 }
 
+// @doc Only for *Local* initiated stream
 static QUIC_STATUS
 handle_stream_event_start_complete(QuicerStreamCTX *s_ctx,
  __unused_parm__ QUIC_STREAM_EVENT *Event)

include/quicer_types.hrl

@@ -223,7 +223,7 @@
  param_conn_datagram_receive_enabled | %% | X | X | @TODO
  param_conn_datagram_send_enabled | %% | X | | @TODO
  param_conn_disable_1rtt_encryption | %% | X | X |
- param_conn_resumption_ticket | %% | | X | @TODO
+ param_conn_resumption_ticket | %% | | X |
  param_conn_peer_certificate_valid | %% | | X | @TODO
  param_conn_local_interface. %% | | X | @TODO
 

rebar.config

@@ -39,6 +39,7 @@
 %% Coveralls
 {plugins , [coveralls]}. % use hex package
 {cover_enabled , true}.
+{cover_excl_mods, [qt, qt_ssl, rev, user_default]}.
 {cover_export_enabled , true}.
 {coveralls_coverdata , "_build/test/cover/*.coverdata"}. % or a string with wildcards or a list of files
 {coveralls_service_name , "github"}.

src/quicer.erl

@@ -213,7 +213,7 @@ close_listener(Listener) ->
  inet:port_number(), conn_opts(), timeout()) ->
  {ok, connection_handle()} |
  {error, conn_open_error | config_error | conn_start_error} |
- {error, timeout}.
+ {error, timeout} | {error, nst_not_found}.
 connect(Host, Port, Opts, Timeout) when is_list(Opts) ->
  connect(Host, Port, maps:from_list(Opts), Timeout);
 connect(Host, Port, Opts, Timeout) when is_tuple(Host) ->
@@ -235,7 +235,10 @@ connect(Host, Port, Opts, Timeout) when is_map(Opts) ->
  {error, transport_down, Reason}
  end;
  {error, _} = Err ->
- Err
+ Err;
+ {error, not_found, _} ->
+ %% nst error
+ {error, nst_not_found}
  end.
 
 %% @doc

src/quicer_nif.erl

@@ -122,7 +122,8 @@ open_connection() ->
 
 -spec async_connect(hostname(), inet:port_number(), conn_opts()) ->
  {ok, connection_handle()} |
- {error, conn_open_error | config_error | conn_start_error}.
+ {error, conn_open_error | config_error | conn_start_error} |
+ {error, not_found, any()}.
 async_connect(_Host, _Port, _Opts) ->
  erlang:nif_error(nif_library_not_loaded).