refactor: refcnt part 2

c_src/quicer_connection.c

@@ -574,11 +574,6 @@ open_connectionX(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
   // It is safe to use r_ctx here since
   // it is passed as argument which beam still has reference to
 
-  if (r_ctx != &G_r_ctx)
-  {
-    enif_keep_resource(r_ctx);
-  }
-
   c_ctx->r_ctx = r_ctx;
   enif_mutex_lock(r_ctx->lock);
 
@@ -621,7 +616,6 @@ open_connectionX(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
   return SUCCESS(eHandle);
 
 exit:
-  enif_release_resource(r_ctx);
   enif_release_resource(c_ctx);
   return res;
 }
@@ -672,11 +666,6 @@ async_connect3(ErlNifEnv *env,
           // r_ctx is already kept in open_connectionX
           r_ctx = c_ctx->r_ctx;
           is_reuse_handle = TRUE;
-          /* // @FIXME: I don't think it is necessary. */
-          /* if (!get_reg_handle(r_ctx)) */
-          /*   { */
-          /*     return ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); */
-          /*   } */
         }
       else
         {
@@ -925,10 +914,6 @@ async_connect3(ErlNifEnv *env,
       MsQuic->ConnectionClose(Connection);
     }
 
-  if (!is_reuse_handle)
-    {
-      enif_release_resource(r_ctx);
-    }
   return res;
 }
 

c_src/quicer_ctx.c

@@ -49,19 +49,6 @@ deinit_r_ctx(QuicerRegistrationCTX *r_ctx)
   enif_mutex_destroy(r_ctx->lock);
 }
 
-void
-destroy_r_ctx(QuicerRegistrationCTX *r_ctx)
-{
-  if (r_ctx == &G_r_ctx)
-  {
-    deinit_r_ctx(r_ctx);
-  }
-  else
-  {
-    enif_release_resource(r_ctx);
-  }
-}
-
 QuicerListenerCTX *
 init_l_ctx()
 {
@@ -99,10 +86,6 @@ deinit_l_ctx(QuicerListenerCTX *l_ctx)
     {
       destroy_config_ctx(l_ctx->config_resource);
     }
-  if (l_ctx->r_ctx)
-    {
-      enif_release_resource(l_ctx->r_ctx);
-    }
   enif_mutex_destroy(l_ctx->lock);
   enif_free_env(l_ctx->env);
 }
@@ -172,6 +155,7 @@ init_c_ctx()
 void
 deinit_c_ctx(QuicerConnCTX *c_ctx)
 {
+  CXPLAT_FRE_ASSERT(!c_ctx->r_ctx);
   enif_free_env(c_ctx->env);
 #if defined(QUICER_USE_TRUSTED_STORE)
   if (c_ctx->trusted != NULL)
@@ -190,12 +174,6 @@ deinit_c_ctx(QuicerConnCTX *c_ctx)
     {
       X509_free(c_ctx->peer_cert);
     }
-
-  if (c_ctx->r_ctx)
-    {
-      put_reg_handle(c_ctx->r_ctx);
-      c_ctx->r_ctx = NULL;
-    }
   enif_mutex_destroy(c_ctx->lock);
 }
 
@@ -376,6 +354,8 @@ put_conn_handle(QuicerConnCTX *c_ctx)
       c_ctx->Connection = NULL;
       c_ctx->is_closed = TRUE;
       MsQuic->SetCallbackHandler(Connection, NULL, NULL);
+      put_reg_handle(c_ctx->r_ctx);
+      c_ctx->r_ctx = NULL;
       MsQuic->ConnectionClose(Connection);
     }
 }
@@ -420,7 +400,14 @@ put_reg_handle(QuicerRegistrationCTX *r_ctx)
     r_ctx->Registration = NULL;
     MsQuic->RegistrationShutdown(Registration, QUIC_CONNECTION_SHUTDOWN_FLAG_NONE, 0);
     MsQuic->RegistrationClose(Registration);
-    destroy_r_ctx(r_ctx);
+    if (r_ctx == &G_r_ctx)
+    {
+      deinit_r_ctx(r_ctx);
+    }
+    else
+    {
+      enif_release_resource(r_ctx);
+    }
   }
 }
 

c_src/quicer_ctx.h

@@ -162,7 +162,6 @@ typedef struct QuicerStreamSendCTX QuicerDgramSendCTX;
 
 QuicerRegistrationCTX *init_r_ctx(QuicerRegistrationCTX *r_ctx);
 void deinit_r_ctx(QuicerRegistrationCTX *r_ctx);
-void destroy_r_ctx(QuicerRegistrationCTX *r_ctx);
 
 QuicerListenerCTX *init_l_ctx();
 void deinit_l_ctx(QuicerListenerCTX *l_ctx);

c_src/quicer_eterms.h

@@ -24,6 +24,7 @@ extern ERL_NIF_TERM ATOM_FALSE;
 // quicer internal 'errors'
 extern ERL_NIF_TERM ATOM_OK;
 extern ERL_NIF_TERM ATOM_ERROR;
+extern ERL_NIF_TERM ATOM_GLOBAL;
 extern ERL_NIF_TERM ATOM_REG_FAILED;
 extern ERL_NIF_TERM ATOM_OPEN_FAILED;
 extern ERL_NIF_TERM ATOM_CTX_INIT_FAILED;

c_src/quicer_listener.c

@@ -43,9 +43,7 @@ ServerListenerCallback(__unused_parm__ HQUIC Listener,
 
   if (is_worker)
     {
-      printf("trying lock!!!\n");
       enif_mutex_lock(l_ctx->lock);
-      printf("trying unlock!!!\n");
     }
 
   switch (Event->Type)
@@ -98,13 +96,16 @@ ServerListenerCallback(__unused_parm__ HQUIC Listener,
           // We are going to reject the connection,
           // we will not be the owner of this connection
           // msquic will close the Connection Handle internally.
-          // Set it to NULL to avoid close it in resource_conn_dealloc_callback
-          // or in the put_conn_handle.
+          // Set MsQuic Handles to NULL to avoid double free in
+          // resource_conn_dealloc_callback
+          // AND
+          // put_*_handle
           c_ctx->Connection = NULL;
+          c_ctx->r_ctx = NULL;
+
           put_conn_handle(c_ctx);
           put_reg_handle(r_ctx);
           CXPLAT_FRE_ASSERTMSG(r_ctx->ref_count > 0, "Listener should still own the r_ctx");
-          c_ctx->r_ctx = NULL;
           // However, we still need to release the c_ctx resource
           // @NOTE: we don't hold the lock of c_ctx since it is new conn.
           // @TODO: the next step should be part of put_conn_handle/1 and need to be tested
@@ -236,8 +237,8 @@ ServerListenerCallback(__unused_parm__ HQUIC Listener,
     case QUIC_LISTENER_EVENT_STOP_COMPLETE:
       // **Note**, this callback event in msquic can be triggered by either
       // MsQuicListenerClose or MsQuicListenerStop.
-      printf("!!!!!!!handle stop complete\n");
       env = l_ctx->env;
+
       enif_send(NULL,
                 &(l_ctx->listenerPid),
                 env,
@@ -683,6 +684,7 @@ get_listenersX(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
       Entry = Entry->Flink;
     }
   enif_mutex_unlock(r_ctx->lock);
+  put_reg_handle(r_ctx);
   return res;
 }
 

c_src/quicer_nif.c

@@ -47,6 +47,7 @@ closeLib(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
 
 ERL_NIF_TERM ATOM_TRUE;
 ERL_NIF_TERM ATOM_FALSE;
+ERL_NIF_TERM ATOM_GLOBAL;
 
 // quicer internal 'errors'
 ERL_NIF_TERM ATOM_OK;
@@ -475,6 +476,7 @@ ERL_NIF_TERM ATOM_QUIC_SEND_ECN_CONGESTION_COUNT;
 #define INIT_ATOMS                                                            \
   ATOM(ATOM_TRUE, true);                                                      \
   ATOM(ATOM_FALSE, false);                                                    \
+  ATOM(ATOM_GLOBAL, global);                                                    \
                                                                               \
   ATOM(ATOM_OK, ok);                                                          \
   ATOM(ATOM_ERROR, error);                                                    \
@@ -1057,12 +1059,10 @@ void
 resource_reg_dealloc_callback(__unused_parm__ ErlNifEnv *env, void *obj)
 {
   TP_CB_3(start, (uintptr_t)obj, 0);
-  QuicerRegistrationCTX *reg_ctx = (QuicerRegistrationCTX *)obj;
-  deinit_r_ctx(reg_ctx);
-  if (MsQuic && reg_ctx->Registration)
-    {
-      //MsQuic->RegistrationClose(reg_ctx->Registration);
-    }
+  QuicerRegistrationCTX *r_ctx = (QuicerRegistrationCTX *)obj;
+  CXPLAT_FRE_ASSERT(r_ctx->ref_count == 0);
+  CXPLAT_FRE_ASSERT(!r_ctx->Registration);
+  deinit_r_ctx(r_ctx);
   TP_CB_3(end, (uintptr_t)obj, 0);
 }
 
@@ -1356,7 +1356,7 @@ closeLib(__unused_parm__ ErlNifEnv *env,
           }
           while(G_r_ctx.ref_count != 2)
             {
-              printf("shutdown lib wait for global reg cnt to be 2\nnow: %ld\n",
+              printf("closelib wait for global reg cnt to be 2 but now: %ld\n",
                      G_r_ctx.ref_count);
             }
           put_reg_handle(&G_r_ctx);
@@ -1766,6 +1766,7 @@ static ErlNifFunc nif_funcs[] = {
   { "count_reg_conns", 0, count_reg_connsX, 0},
   { "count_reg_conns", 1, count_reg_connsX, 0},
   /* for DEBUG */
+  { "get_registration_refcnt", 1, get_registration_refcnt, 0},
   { "get_conn_rid", 1, get_conn_rid1, 1},
   { "get_stream_rid", 1, get_stream_rid1, 1},
   { "get_listeners", 0, get_listenersX, 0},

c_src/quicer_reg.c

@@ -19,7 +19,7 @@ limitations under the License.
 static BOOLEAN parse_reg_conf(ERL_NIF_TERM eprofile,
                               QUIC_REGISTRATION_CONFIG *RegConfig);
 
-QuicerRegistrationCTX G_r_ctx = {.is_released = TRUE};
+QuicerRegistrationCTX G_r_ctx = {.name = "global", .is_released = TRUE};
 pthread_mutex_t GRegLock = PTHREAD_MUTEX_INITIALIZER;
 extern pthread_mutex_t MsQuicLock;
 
@@ -136,21 +136,20 @@ new_registration2(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
           || strlen(r_ctx->name) == 0))
     {
       res = ERROR_TUPLE_2(ATOM_BADARG);
-      goto exit;
+      goto err_exit;
     }
 
   RegConfig.AppName = r_ctx->name;
   if (QUIC_FAILED(
           status = MsQuic->RegistrationOpen(&RegConfig, &r_ctx->Registration)))
     {
       res = ERROR_TUPLE_2(ATOM_STATUS(status));
-      goto exit;
+      goto err_exit;
     }
-  CxPlatRefInitialize(&r_ctx->ref_count);
   return SUCCESS(enif_make_resource(env, r_ctx));
 
-exit:
-  destroy_r_ctx(r_ctx);
+err_exit:
+  enif_release_resource(r_ctx);
   return res;
 }
 
@@ -214,7 +213,7 @@ close_registration(ErlNifEnv *env,
   r_ctx->Registration = NULL;
   enif_mutex_unlock(r_ctx->lock);
   MsQuic->RegistrationClose(Registration);
-  destroy_r_ctx(r_ctx);
+  put_reg_handle(r_ctx);
   return ATOM_OK;
 }
 
@@ -236,6 +235,31 @@ get_registration_name1(ErlNifEnv *env,
   return SUCCESS(name);
 }
 
+ERL_NIF_TERM
+get_registration_refcnt(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM *argv)
+{
+  QuicerRegistrationCTX *r_ctx = NULL;
+  ERL_NIF_TERM ectx = argv[0];
+  CXPLAT_DBG_ASSERT(argc == 1);
+
+  if (IS_SAME_TERM(ectx, ATOM_GLOBAL))
+    {
+      r_ctx = &G_r_ctx;
+    }
+  else if (!enif_get_resource(env, ectx, ctx_reg_t, (void **)&r_ctx))
+    {
+      return ERROR_TUPLE_2(ATOM_BADARG);
+    }
+
+  if (!get_reg_handle(r_ctx))
+    {
+      return ERROR_TUPLE_2(ATOM_CLOSED);
+    }
+  CXPLAT_REF_COUNT cnt = r_ctx->ref_count;
+  put_reg_handle(r_ctx);
+  return enif_make_int64(env, cnt-1);
+}
+
 BOOLEAN
 parse_reg_conf(ERL_NIF_TERM eprofile, QUIC_REGISTRATION_CONFIG *RegConfig)
 {

c_src/quicer_reg.h

@@ -35,4 +35,6 @@ close_registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
 ERL_NIF_TERM
 get_registration_name1(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
 
+ERL_NIF_TERM
+get_registration_refcnt(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]);
 #endif // QUICER_REG_H_

src/quicer.erl

@@ -28,6 +28,7 @@
     shutdown_registration/1,
     shutdown_registration/3,
     get_registration_name/1,
+    get_registration_refcnt/1,
     reg_open/0,
     reg_open/1,
     reg_close/0
@@ -239,6 +240,12 @@ close_registration(Handle) ->
 get_registration_name(Handle) ->
     quicer_nif:get_registration_name(Handle).
 
+%% @doc get registration reference count
+-spec get_registration_refcnt(global | reg_handle()) ->
+    quicer_nif:get_registration_refcnt().
+get_registration_refcnt(Handle) ->
+    quicer_nif:get_registration_refcnt(Handle).
+
 %% @doc GRegistraion should be opened before calling traffic APIs.
 %%
 %% This is called automatically when quicer application starts with

src/quicer_listener.erl

@@ -220,6 +220,8 @@ handle_cast(_Request, State) ->
     | {noreply, NewState :: term(), hibernate}
     | {stop, Reason :: normal | term(), NewState :: term()}.
 handle_info({quic, listener_stopped, L}, #state{listener = L} = State) ->
+    %% uncontroled stop:
+    _ = quicer:close_listener(L),
     {stop, normal, State};
 handle_info(_Info, State) ->
     {noreply, State}.

src/quicer_nif.erl

@@ -25,6 +25,7 @@
     shutdown_registration/3,
     close_registration/1,
     get_registration_name/1,
+    get_registration_refcnt/1,
     listen/2,
     start_listener/3,
     stop_listener/1,
@@ -90,6 +91,7 @@
     shutdown_registration/0,
     close_registration/0,
     get_registration_name/0,
+    get_registration_refcnt/0,
     get_listeners/0,
     get_connections/0,
     get_owner/0,
@@ -103,6 +105,7 @@
 -type shutdown_registration() :: ok | {error, badarg}.
 -type close_registration() :: ok | {error, badarg}.
 -type get_registration_name() :: {ok, string()} | {error, badarg}.
+-type get_registration_refcnt() :: {error, closed} | integer().
 -type get_listeners() :: [listener_handle()].
 -type get_connections() :: [connection_handle()].
 -type get_owner() :: {ok, pid()} | {error, undefined | badarg}.
@@ -219,6 +222,10 @@ close_registration(_Handle) ->
 get_registration_name(_Handle) ->
     erlang:nif_error(nif_library_not_loaded).
 
+-spec get_registration_refcnt(reg_handle()) -> get_registration_refcnt().
+get_registration_refcnt(_Handle) ->
+    erlang:nif_error(nif_library_not_loaded).
+
 -spec listen(listen_on(), listen_opts()) ->
     {ok, listener_handle()}
     | {error, listener_open_error, atom_reason()}