From 9f35c658e0ef3361de294e85afbcc6aeae286731 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 25 Sep 2023 14:23:19 +0200 Subject: [PATCH 1/6] feat(reg): intro G_r_ctx for global registration --- c_src/quicer_config.c | 6 +- c_src/quicer_connection.c | 38 ++++++++- c_src/quicer_ctx.c | 6 +- c_src/quicer_ctx.h | 2 + c_src/quicer_listener.c | 12 ++- c_src/quicer_nif.c | 89 +++---------------- c_src/quicer_nif.h | 1 - c_src/quicer_reg.c | 174 ++++++++++++++++++++++++++++---------- c_src/quicer_reg.h | 6 ++ src/quicer_app.erl | 2 +- test/quicer_SUITE.erl | 13 ++- 11 files changed, 211 insertions(+), 138 deletions(-) diff --git a/c_src/quicer_config.c b/c_src/quicer_config.c index 31a35c1c..f970873f 100644 --- a/c_src/quicer_config.c +++ b/c_src/quicer_config.c @@ -20,7 +20,7 @@ limitations under the License. #include "quicer_tls.h" #include -extern BOOLEAN isRegistered; +extern QuicerRegistrationCTX *G_r_ctx; static ERL_NIF_TERM get_stream_opt(ErlNifEnv *env, QuicerStreamCTX *s_ctx, @@ -217,7 +217,7 @@ ServerLoadConfiguration(ErlNifEnv *env, { QUIC_SETTINGS Settings = { 0 }; - if (!isRegistered && (Registration == GRegistration)) + if (!G_r_ctx) { return ATOM_REG_FAILED; } @@ -273,7 +273,7 @@ ClientLoadConfiguration(ErlNifEnv *env, QUIC_SETTINGS Settings = { 0 }; ERL_NIF_TERM ret = ATOM_OK; - if (!isRegistered && (Registration == GRegistration)) + if (!G_r_ctx) { return ATOM_REG_FAILED; } diff --git a/c_src/quicer_connection.c b/c_src/quicer_connection.c index 4bccc9e4..d392466d 100644 --- a/c_src/quicer_connection.c +++ b/c_src/quicer_connection.c @@ -23,6 +23,8 @@ limitations under the License. #include #include +extern QuicerRegistrationCTX *G_r_ctx; + #ifdef DEBUG extern inline void EncodeHexBuffer(uint8_t *Buffer, uint8_t BufferLen, char *HexString); @@ -505,7 +507,14 @@ open_connectionX(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) if (argc == 0) { - registration = GRegistration; + if (G_r_ctx) + { + registration = G_r_ctx->Registration; + } + else + { + return ERROR_TUPLE_2(ATOM_REG_FAILED); + } r_ctx = NULL; } else @@ -522,7 +531,14 @@ open_connectionX(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) } else { - registration = GRegistration; + if (G_r_ctx) + { + registration = G_r_ctx->Registration; + } + else + { + return ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); + } } } @@ -619,7 +635,14 @@ async_connect3(ErlNifEnv *env, } else { - Registration = GRegistration; + if (G_r_ctx) + { + Registration = G_r_ctx->Registration; + } + else + { + return ERROR_TUPLE_2(ATOM_REG_FAILED); + } } } else @@ -649,7 +672,14 @@ async_connect3(ErlNifEnv *env, { // quic_registration is not set, use global registration // msquic should reject if global registration is NULL (closed) - Registration = GRegistration; + if (G_r_ctx && G_r_ctx->Registration) + { + Registration = G_r_ctx->Registration; + } + else + { + Registration = NULL; + } } if ((c_ctx->owner = AcceptorAlloc()) == NULL) diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index b5c376c6..2bdf3402 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -17,7 +17,7 @@ limitations under the License. #include "quicer_ctx.h" // alloc/dealloc ctx should be done in the callbacks. - +extern QuicerRegistrationCTX *G_r_ctx; QuicerRegistrationCTX * init_r_ctx() { @@ -31,6 +31,8 @@ init_r_ctx() r_ctx->env = enif_alloc_env(); r_ctx->Registration = NULL; r_ctx->is_released = FALSE; + CxPlatListInitializeHead(&r_ctx->Listeners); + CxPlatListInitializeHead(&r_ctx->Connections); return r_ctx; } @@ -81,7 +83,7 @@ deinit_l_ctx(QuicerListenerCTX *l_ctx) { destroy_config_ctx(l_ctx->config_resource); } - if (l_ctx->r_ctx && l_ctx->r_ctx->Registration != GRegistration) + if (l_ctx->r_ctx && l_ctx->r_ctx != G_r_ctx) { enif_release_resource(l_ctx->r_ctx); } diff --git a/c_src/quicer_ctx.h b/c_src/quicer_ctx.h index 531d2fcc..254ef417 100644 --- a/c_src/quicer_ctx.h +++ b/c_src/quicer_ctx.h @@ -36,6 +36,8 @@ typedef struct QuicerRegistrationCTX HQUIC Registration; BOOLEAN is_released; char name[UINT8_MAX + 1]; + CXPLAT_LIST_ENTRY Listeners; + CXPLAT_LIST_ENTRY Connections; } QuicerRegistrationCTX; /* diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index f8fdfe78..d8e2a386 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -22,6 +22,8 @@ limitations under the License. #include #include +extern QuicerRegistrationCTX *G_r_ctx; + BOOLEAN parse_registration(ErlNifEnv *env, ERL_NIF_TERM options, QuicerRegistrationCTX **r_ctx); @@ -325,7 +327,15 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) { // quic_registration is not set, use global registration // msquic should reject if global registration is NULL (closed) - Registration = GRegistration; + if (G_r_ctx) + { + Registration = G_r_ctx->Registration; + } + else + { + ret = ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); + goto exit; + } } // Now load server config diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 11f3a2ed..661b53f2 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -768,12 +768,12 @@ ERL_NIF_TERM ATOM_QUIC_DATAGRAM_SEND_CANCELED; ATOM(ATOM_QUIC_DATAGRAM_SEND_CANCELED, dgram_send_canceled); \ ATOM(ATOM_UNDEFINED, undefined); -HQUIC GRegistration = NULL; +extern QuicerRegistrationCTX *G_r_ctx; + const QUIC_API_TABLE *MsQuic = NULL; // @todo, these flags are not threads safe, wrap it in a context -BOOLEAN isRegistered = false; -BOOLEAN isLibOpened = false; +BOOLEAN isLibOpened = FALSE; ErlNifResourceType *ctx_reg_t = NULL; ErlNifResourceType *ctx_listener_t = NULL; @@ -935,7 +935,7 @@ resource_config_dealloc_callback(__unused_parm__ ErlNifEnv *env, TP_CB_3(start, (uintptr_t)obj, 0); QuicerConfigCTX *config_ctx = (QuicerConfigCTX *)obj; // Check if Registration is closed or not - if (GRegistration && config_ctx->Configuration) + if (G_r_ctx && config_ctx->Configuration) { MsQuic->ConfigurationClose(config_ctx->Configuration); } @@ -1048,13 +1048,12 @@ on_upgrade(ErlNifEnv *env, static void on_unload(__unused_parm__ ErlNifEnv *env, __unused_parm__ void *priv_data) { - // @TODO We want registration context and APIs for it - if (isRegistered) + if (G_r_ctx) { - MsQuic->RegistrationClose(GRegistration); - isRegistered = FALSE; + // @TODO memleak here + MsQuic->RegistrationClose(G_r_ctx->Registration); + G_r_ctx = NULL; } - if (isLibOpened) { MsQuicClose(MsQuic); @@ -1087,11 +1086,11 @@ openLib(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // if (QUIC_FAILED(status = MsQuicOpen2(&MsQuic))) { - isLibOpened = false; + isLibOpened = FALSE; return ERROR_TUPLE_3(ATOM_OPEN_FAILED, ATOM_STATUS(status)); } - isLibOpened = true; + isLibOpened = TRUE; TP_NIF_3(success, 0, 2); res = ATOM_TRUE; @@ -1125,74 +1124,6 @@ closeLib(__unused_parm__ ErlNifEnv *env, return ATOM_OK; } -/* -** For global registration only -*/ -static ERL_NIF_TERM -registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) -{ - QUIC_STATUS status = QUIC_STATUS_SUCCESS; - ERL_NIF_TERM profile = argv[0]; - - if (isRegistered || !isLibOpened) - { - return ERROR_TUPLE_2(ATOM_BADARG); - } - - if (argc == 1) - { - if (IS_SAME_TERM(profile, ATOM_QUIC_EXECUTION_PROFILE_LOW_LATENCY)) - { - GRegConfig.ExecutionProfile = QUIC_EXECUTION_PROFILE_LOW_LATENCY; - } - else if (IS_SAME_TERM(profile, - ATOM_QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT)) - { - GRegConfig.ExecutionProfile - = QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT; - } - else if (IS_SAME_TERM(profile, - ATOM_QUIC_EXECUTION_PROFILE_TYPE_SCAVENGER)) - { - GRegConfig.ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_SCAVENGER; - } - else if (IS_SAME_TERM(profile, - ATOM_QUIC_EXECUTION_PROFILE_TYPE_REAL_TIME)) - { - GRegConfig.ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_REAL_TIME; - } - else - { - return ERROR_TUPLE_2(ATOM_BADARG); - } - } - // Open global registration - if (QUIC_FAILED(status - = MsQuic->RegistrationOpen(&GRegConfig, &GRegistration))) - { - isRegistered = false; - TP_NIF_3(fail, 0, status); - return ERROR_TUPLE_3(ATOM_REG_FAILED, ETERM_INT(status)); - } - TP_NIF_3(success, 0, status); - isRegistered = true; - return ATOM_OK; -} - -static ERL_NIF_TERM -deregistration(__unused_parm__ ErlNifEnv *env, - __unused_parm__ int argc, - __unused_parm__ const ERL_NIF_TERM argv[]) -{ - if (isRegistered && GRegistration) - { - MsQuic->RegistrationClose(GRegistration); - GRegistration = NULL; - isRegistered = false; - } - return ATOM_OK; -} - ERL_NIF_TERM atom_status(ErlNifEnv *env, QUIC_STATUS status) { diff --git a/c_src/quicer_nif.h b/c_src/quicer_nif.h index cf2b1686..b7ed558f 100644 --- a/c_src/quicer_nif.h +++ b/c_src/quicer_nif.h @@ -41,7 +41,6 @@ limitations under the License. // Global registration // @todo avoid use globals -extern HQUIC GRegistration; extern const QUIC_API_TABLE *MsQuic; extern QUIC_REGISTRATION_CONFIG GRegConfig; diff --git a/c_src/quicer_reg.c b/c_src/quicer_reg.c index 20b03ec8..05ec9841 100644 --- a/c_src/quicer_reg.c +++ b/c_src/quicer_reg.c @@ -16,41 +16,34 @@ limitations under the License. #include "quicer_reg.h" #include "quicer_nif.h" +static BOOLEAN parse_reg_conf(ERL_NIF_TERM eprofile, + QUIC_REGISTRATION_CONFIG *RegConfig); + +extern BOOLEAN isLibOpened; +QuicerRegistrationCTX *G_r_ctx = NULL; + +/* +** For global registration only +*/ ERL_NIF_TERM -new_registration2(ErlNifEnv *env, - __unused_parm__ int argc, - const ERL_NIF_TERM argv[]) +registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { - QUIC_STATUS status = QUIC_STATUS_SUCCESS; - ERL_NIF_TERM ename = argv[0]; + ERL_NIF_TERM eprofile = ATOM_UNDEFINED; QUIC_REGISTRATION_CONFIG RegConfig - = { NULL, QUIC_EXECUTION_PROFILE_LOW_LATENCY }; + = { "global", QUIC_EXECUTION_PROFILE_LOW_LATENCY }; + QUIC_STATUS status; + ERL_NIF_TERM res = ATOM_OK; - TP_NIF_3(start, 0, status); - if (argc == 2) + // if (!isLibOpened || G_r_ctx) + if (!isLibOpened || G_r_ctx) { - ERL_NIF_TERM eprofile = argv[1]; - if (IS_SAME_TERM(eprofile, ATOM_QUIC_EXECUTION_PROFILE_LOW_LATENCY)) - { - RegConfig.ExecutionProfile = QUIC_EXECUTION_PROFILE_LOW_LATENCY; - } - else if (IS_SAME_TERM(eprofile, - ATOM_QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT)) - { - RegConfig.ExecutionProfile - = QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT; - } - else if (IS_SAME_TERM(eprofile, - ATOM_QUIC_EXECUTION_PROFILE_TYPE_SCAVENGER)) - { - RegConfig.ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_SCAVENGER; - } - else if (IS_SAME_TERM(eprofile, - ATOM_QUIC_EXECUTION_PROFILE_TYPE_REAL_TIME)) - { - RegConfig.ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_REAL_TIME; - } - else + return ERROR_TUPLE_2(ATOM_BADARG); + } + + if (argc == 1) + { + eprofile = argv[0]; + if (!parse_reg_conf(eprofile, &RegConfig)) { return ERROR_TUPLE_2(ATOM_BADARG); } @@ -59,29 +52,97 @@ new_registration2(ErlNifEnv *env, QuicerRegistrationCTX *r_ctx = init_r_ctx(); if (!r_ctx) { - ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); + return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); + } + + if (QUIC_FAILED( + status = MsQuic->RegistrationOpen(&RegConfig, &r_ctx->Registration))) + { + res = ERROR_TUPLE_2(ATOM_STATUS(status)); + goto exit; + } + + // Keep global registration context + // enif_keep_resource(r_ctx); + G_r_ctx = r_ctx; + return ATOM_OK; + +exit: + deinit_r_ctx(r_ctx); + destroy_r_ctx(r_ctx); + return res; +} + +/* +** For global registration only +*/ +ERL_NIF_TERM +deregistration(__unused_parm__ ErlNifEnv *env, + __unused_parm__ int argc, + __unused_parm__ const ERL_NIF_TERM argv[]) +{ + int error_code = 0; + if (!isLibOpened) + { + return ERROR_TUPLE_2(ATOM_BADARG); } - if (0 >= enif_get_string( - env, ename, r_ctx->name, UINT8_MAX + 1, ERL_NIF_LATIN1)) + if (G_r_ctx) { - deinit_r_ctx(r_ctx); - destroy_r_ctx(r_ctx); - ERROR_TUPLE_2(ATOM_BADARG); + MsQuic->RegistrationShutdown(G_r_ctx->Registration, FALSE, error_code); + G_r_ctx->Registration = NULL; + destroy_r_ctx(G_r_ctx); + G_r_ctx = NULL; + } + + return ATOM_OK; +} + +ERL_NIF_TERM +new_registration2(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) +{ + + ERL_NIF_TERM ename = argv[0]; + ERL_NIF_TERM eprofile = argv[1]; + QUIC_REGISTRATION_CONFIG RegConfig + = { NULL, QUIC_EXECUTION_PROFILE_LOW_LATENCY }; + + QUIC_STATUS status; + ERL_NIF_TERM res = ATOM_OK; + + if (!parse_reg_conf(eprofile, &RegConfig)) + { + return ERROR_TUPLE_2(ATOM_BADARG); + } + + QuicerRegistrationCTX *r_ctx = init_r_ctx(); + if (!r_ctx) + { + return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); + } + + if (argc == 2 + && 0 >= enif_get_string( + env, ename, r_ctx->name, UINT8_MAX + 1, ERL_NIF_LATIN1)) + { + res = ERROR_TUPLE_2(ATOM_BADARG); + goto exit; } - // Open Registration + RegConfig.AppName = r_ctx->name; if (QUIC_FAILED( status = MsQuic->RegistrationOpen(&RegConfig, &r_ctx->Registration))) { - // unlikely - TP_NIF_3(fail, 0, status); - deinit_r_ctx(r_ctx); - destroy_r_ctx(r_ctx); - return ERROR_TUPLE_2(ATOM_STATUS(status)); + res = ERROR_TUPLE_2(ATOM_STATUS(status)); + goto exit; } - TP_NIF_3(success, 0, status); + return SUCCESS(enif_make_resource(env, r_ctx)); + +exit: + deinit_r_ctx(r_ctx); + destroy_r_ctx(r_ctx); + return res; } ERL_NIF_TERM @@ -142,3 +203,30 @@ get_registration_name1(ErlNifEnv *env, return SUCCESS(enif_make_string(env, r_ctx->name, ERL_NIF_LATIN1)); } + +BOOLEAN +parse_reg_conf(ERL_NIF_TERM eprofile, QUIC_REGISTRATION_CONFIG *RegConfig) +{ + if (IS_SAME_TERM(eprofile, ATOM_QUIC_EXECUTION_PROFILE_LOW_LATENCY)) + { + RegConfig->ExecutionProfile = QUIC_EXECUTION_PROFILE_LOW_LATENCY; + } + else if (IS_SAME_TERM(eprofile, + ATOM_QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT)) + { + RegConfig->ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT; + } + else if (IS_SAME_TERM(eprofile, ATOM_QUIC_EXECUTION_PROFILE_TYPE_SCAVENGER)) + { + RegConfig->ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_SCAVENGER; + } + else if (IS_SAME_TERM(eprofile, ATOM_QUIC_EXECUTION_PROFILE_TYPE_REAL_TIME)) + { + RegConfig->ExecutionProfile = QUIC_EXECUTION_PROFILE_TYPE_REAL_TIME; + } + else + { + return FALSE; + } + return TRUE; +} diff --git a/c_src/quicer_reg.h b/c_src/quicer_reg.h index 38444423..c326530d 100644 --- a/c_src/quicer_reg.h +++ b/c_src/quicer_reg.h @@ -17,6 +17,12 @@ limitations under the License. #define QUICER_REG_H_ #include +ERL_NIF_TERM +registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); + +ERL_NIF_TERM +deregistration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); + ERL_NIF_TERM new_registration2(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); diff --git a/src/quicer_app.erl b/src/quicer_app.erl index 13be524e..0b88baf4 100644 --- a/src/quicer_app.erl +++ b/src/quicer_app.erl @@ -22,7 +22,7 @@ start(_StartType, _StartArgs) -> quicer:open_lib(), - quicer:reg_open(application:get_env(quicer, profile, quic_execution_profile_low_latency)), + _ = quicer:reg_open(application:get_env(quicer, profile, quic_execution_profile_low_latency)), quicer_sup:start_link(). stop(_) -> diff --git a/test/quicer_SUITE.erl b/test/quicer_SUITE.erl index 07204289..eb43f4a1 100644 --- a/test/quicer_SUITE.erl +++ b/test/quicer_SUITE.erl @@ -33,7 +33,7 @@ end_per_testcase/2]). %% test cases --export([ tc_nif_module_load/1 +-export([tc_nif_module_load/1 , tc_nif_module_unload/1 , tc_nif_module_reload/1 , tc_open_lib_test/1 @@ -271,11 +271,16 @@ tc_close_lib_test(_Config) -> tc_lib_registration_neg(_Config) -> ok = quicer:close_lib(), {error, badarg} = quicer:reg_open(), - ok = quicer:reg_close(). + {error, badarg} = quicer:reg_close(). tc_lib_registration(_Config) -> quicer:open_lib(), - ok = quicer:reg_open(), + case quicer:reg_open() of + {error, badarg} -> + quicer:reg_close(); + ok -> + ok + end, ok = quicer:reg_close(). tc_lib_registration_1(_Config) -> @@ -306,7 +311,7 @@ tc_open_listener_inval_reg(Config) -> Port = select_port(), ok = quicer:reg_close(), ok = quicer:close_lib(), - {error, config_error, reg_failed} = quicer:listen(Port, default_listen_opts(Config)), + {error, quic_registration} = quicer:listen(Port, default_listen_opts(Config)), quicer:open_lib(), quicer:reg_open(), ok. From ee588f6ea4a60e72c2b6df145b17493862ca9466 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 25 Sep 2023 21:49:57 +0200 Subject: [PATCH 2/6] fix: thread safe open/close lib --- c_src/quicer_nif.c | 60 ++++++++++++++++++++++++++++--------------- c_src/quicer_reg.c | 7 ++--- test/quicer_SUITE.erl | 3 +-- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 661b53f2..c82a8d12 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -771,9 +771,8 @@ ERL_NIF_TERM ATOM_QUIC_DATAGRAM_SEND_CANCELED; extern QuicerRegistrationCTX *G_r_ctx; const QUIC_API_TABLE *MsQuic = NULL; - -// @todo, these flags are not threads safe, wrap it in a context -BOOLEAN isLibOpened = FALSE; +// Mutex for MsQuic +ErlNifMutex *MsQuicLock = NULL; ErlNifResourceType *ctx_reg_t = NULL; ErlNifResourceType *ctx_listener_t = NULL; @@ -967,6 +966,11 @@ on_load(ErlNifEnv *env, { int ret_val = 0; + if (!MsQuicLock) + { + MsQuicLock = enif_mutex_create("msquic_lock"); + } + // init atoms in use. #define ATOM(name, val) \ { \ @@ -1054,11 +1058,13 @@ on_unload(__unused_parm__ ErlNifEnv *env, __unused_parm__ void *priv_data) MsQuic->RegistrationClose(G_r_ctx->Registration); G_r_ctx = NULL; } - if (isLibOpened) + if (MsQuic) { MsQuicClose(MsQuic); - isLibOpened = FALSE; + MsQuic = NULL; } + // @TODO memleak here + //enif_mutex_destroy(MsQuicLock); } static ERL_NIF_TERM @@ -1070,11 +1076,16 @@ openLib(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) ERL_NIF_TERM res = ATOM_FALSE; ERL_NIF_TERM lttngLib = argv[0]; char lttngPath[PATH_MAX] = { 0 }; - - if (isLibOpened) + if (MsQuicLock == NULL) + { + return ATOM_OK; + } + enif_mutex_lock(MsQuicLock); + if (MsQuic) { TP_NIF_3(skip, 0, 2); - return SUCCESS(res); + res = SUCCESS(res); + goto exit; } // @todo external call for static link @@ -1086,25 +1097,27 @@ openLib(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // if (QUIC_FAILED(status = MsQuicOpen2(&MsQuic))) { - isLibOpened = FALSE; - return ERROR_TUPLE_3(ATOM_OPEN_FAILED, ATOM_STATUS(status)); + MsQuic = NULL; + res = ERROR_TUPLE_3(ATOM_OPEN_FAILED, ATOM_STATUS(status)); + goto exit; } - isLibOpened = TRUE; TP_NIF_3(success, 0, 2); - res = ATOM_TRUE; + res = SUCCESS(ATOM_TRUE); if (enif_get_string(env, lttngLib, lttngPath, PATH_MAX, ERL_NIF_LATIN1)) { // loading lttng lib is optional, ok to fail if (dlopen(lttngPath, (unsigned)RTLD_NOW | (unsigned)RTLD_GLOBAL)) { - res = ATOM_DEBUG; + res = SUCCESS(ATOM_DEBUG); } } - return SUCCESS(res); +exit: + enif_mutex_unlock(MsQuicLock); + return res; } static ERL_NIF_TERM @@ -1112,15 +1125,22 @@ closeLib(__unused_parm__ ErlNifEnv *env, __unused_parm__ int argc, __unused_parm__ const ERL_NIF_TERM argv[]) { - if (isLibOpened && MsQuic) + if (MsQuicLock == NULL) + { + return ATOM_OK; + } + enif_mutex_lock(MsQuicLock); + if (MsQuic) { - // @todo ensure registration is closed first - // - TP_NIF_3(do_close, 0, isLibOpened); + TP_NIF_3(do_close, MsQuic, 0); + if (G_r_ctx) + { + deregistration(env, argc, argv); + } MsQuicClose(MsQuic); - isLibOpened = false; + MsQuic = NULL; } - + enif_mutex_unlock(MsQuicLock); return ATOM_OK; } diff --git a/c_src/quicer_reg.c b/c_src/quicer_reg.c index 05ec9841..6d6f5aaa 100644 --- a/c_src/quicer_reg.c +++ b/c_src/quicer_reg.c @@ -19,7 +19,6 @@ limitations under the License. static BOOLEAN parse_reg_conf(ERL_NIF_TERM eprofile, QUIC_REGISTRATION_CONFIG *RegConfig); -extern BOOLEAN isLibOpened; QuicerRegistrationCTX *G_r_ctx = NULL; /* @@ -34,8 +33,7 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) QUIC_STATUS status; ERL_NIF_TERM res = ATOM_OK; - // if (!isLibOpened || G_r_ctx) - if (!isLibOpened || G_r_ctx) + if (!MsQuic || G_r_ctx) { return ERROR_TUPLE_2(ATOM_BADARG); } @@ -82,7 +80,7 @@ deregistration(__unused_parm__ ErlNifEnv *env, __unused_parm__ const ERL_NIF_TERM argv[]) { int error_code = 0; - if (!isLibOpened) + if (!MsQuic) { return ERROR_TUPLE_2(ATOM_BADARG); } @@ -90,7 +88,6 @@ deregistration(__unused_parm__ ErlNifEnv *env, if (G_r_ctx) { MsQuic->RegistrationShutdown(G_r_ctx->Registration, FALSE, error_code); - G_r_ctx->Registration = NULL; destroy_r_ctx(G_r_ctx); G_r_ctx = NULL; } diff --git a/test/quicer_SUITE.erl b/test/quicer_SUITE.erl index eb43f4a1..75e68af3 100644 --- a/test/quicer_SUITE.erl +++ b/test/quicer_SUITE.erl @@ -260,8 +260,8 @@ tc_open_lib_test(_Config) -> {ok, false} = quicer:open_lib(). tc_close_lib_test(_Config) -> + ok = quicer:reg_close(), {ok, false} = quicer:open_lib(), - %% @todo close reg before close lib ok = quicer:reg_close(), ok = quicer:close_lib(), ok = quicer:close_lib(), @@ -310,7 +310,6 @@ tc_lib_re_registration(_Config) -> tc_open_listener_inval_reg(Config) -> Port = select_port(), ok = quicer:reg_close(), - ok = quicer:close_lib(), {error, quic_registration} = quicer:listen(Port, default_listen_opts(Config)), quicer:open_lib(), quicer:reg_open(), From ed2e4d4bbb8cce62904eaafeb74271223fedcbaa Mon Sep 17 00:00:00 2001 From: William Yang Date: Tue, 26 Sep 2023 13:51:49 +0200 Subject: [PATCH 3/6] fix: thread safe msquic API table and global registration --- c_src/quicer_connection.c | 2 +- c_src/quicer_ctx.c | 3 +- c_src/quicer_ctx.h | 2 -- c_src/quicer_eterms.h | 1 + c_src/quicer_nif.c | 64 +++++++++++++++++++++++---------------- c_src/quicer_reg.c | 22 +++++++++----- test/quicer_SUITE.erl | 10 ++++-- 7 files changed, 62 insertions(+), 42 deletions(-) diff --git a/c_src/quicer_connection.c b/c_src/quicer_connection.c index d392466d..a3956397 100644 --- a/c_src/quicer_connection.c +++ b/c_src/quicer_connection.c @@ -513,7 +513,7 @@ open_connectionX(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) } else { - return ERROR_TUPLE_2(ATOM_REG_FAILED); + return ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); } r_ctx = NULL; } diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index 2bdf3402..910d25a4 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -18,6 +18,7 @@ limitations under the License. // alloc/dealloc ctx should be done in the callbacks. extern QuicerRegistrationCTX *G_r_ctx; + QuicerRegistrationCTX * init_r_ctx() { @@ -31,8 +32,6 @@ init_r_ctx() r_ctx->env = enif_alloc_env(); r_ctx->Registration = NULL; r_ctx->is_released = FALSE; - CxPlatListInitializeHead(&r_ctx->Listeners); - CxPlatListInitializeHead(&r_ctx->Connections); return r_ctx; } diff --git a/c_src/quicer_ctx.h b/c_src/quicer_ctx.h index 254ef417..531d2fcc 100644 --- a/c_src/quicer_ctx.h +++ b/c_src/quicer_ctx.h @@ -36,8 +36,6 @@ typedef struct QuicerRegistrationCTX HQUIC Registration; BOOLEAN is_released; char name[UINT8_MAX + 1]; - CXPLAT_LIST_ENTRY Listeners; - CXPLAT_LIST_ENTRY Connections; } QuicerRegistrationCTX; /* diff --git a/c_src/quicer_eterms.h b/c_src/quicer_eterms.h index c0b6d2fc..7a5d6e1d 100644 --- a/c_src/quicer_eterms.h +++ b/c_src/quicer_eterms.h @@ -35,6 +35,7 @@ extern ERL_NIF_TERM ATOM_BAD_MON; extern ERL_NIF_TERM ATOM_LISTENER_OPEN_ERROR; extern ERL_NIF_TERM ATOM_LISTENER_START_ERROR; extern ERL_NIF_TERM ATOM_BADARG; +extern ERL_NIF_TERM ATOM_LIB_UNINITIALIZED; extern ERL_NIF_TERM ATOM_CONN_OPEN_ERROR; extern ERL_NIF_TERM ATOM_CONN_START_ERROR; extern ERL_NIF_TERM ATOM_STREAM_OPEN_ERROR; diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index c82a8d12..b11482ce 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -33,6 +33,9 @@ static ERL_NIF_TERM stream_controlling_process(ErlNifEnv *env, const ErlNifPid *caller, const ERL_NIF_TERM *pid); +static ERL_NIF_TERM +closeLib(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); + /* ** atoms in use, initialized while load nif */ @@ -54,6 +57,7 @@ ERL_NIF_TERM ATOM_BAD_MON; ERL_NIF_TERM ATOM_LISTENER_OPEN_ERROR; ERL_NIF_TERM ATOM_LISTENER_START_ERROR; ERL_NIF_TERM ATOM_BADARG; +ERL_NIF_TERM ATOM_LIB_UNINITIALIZED; ERL_NIF_TERM ATOM_CONN_OPEN_ERROR; ERL_NIF_TERM ATOM_CONN_START_ERROR; ERL_NIF_TERM ATOM_STREAM_OPEN_ERROR; @@ -430,6 +434,7 @@ ERL_NIF_TERM ATOM_QUIC_DATAGRAM_SEND_CANCELED; ATOM(ATOM_LISTENER_OPEN_ERROR, listener_open_error); \ ATOM(ATOM_LISTENER_START_ERROR, listener_start_error); \ ATOM(ATOM_BADARG, badarg); \ + ATOM(ATOM_LIB_UNINITIALIZED, lib_uninitialized); \ ATOM(ATOM_CONN_OPEN_ERROR, conn_open_error); \ ATOM(ATOM_CONN_START_ERROR, conn_start_error); \ ATOM(ATOM_STREAM_OPEN_ERROR, stm_open_error); \ @@ -769,6 +774,7 @@ ERL_NIF_TERM ATOM_QUIC_DATAGRAM_SEND_CANCELED; ATOM(ATOM_UNDEFINED, undefined); extern QuicerRegistrationCTX *G_r_ctx; +extern ErlNifMutex *GRegLock; const QUIC_API_TABLE *MsQuic = NULL; // Mutex for MsQuic @@ -948,7 +954,7 @@ 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 (reg_ctx->Registration) + if (MsQuic && reg_ctx->Registration) { MsQuic->RegistrationClose(reg_ctx->Registration); } @@ -966,10 +972,16 @@ on_load(ErlNifEnv *env, { int ret_val = 0; + // Library initialization, library scope if (!MsQuicLock) - { - MsQuicLock = enif_mutex_create("msquic_lock"); - } + { + MsQuicLock = enif_mutex_create("msquic_lock"); + } + + if (!GRegLock) + { + GRegLock = enif_mutex_create("global_reg_lock"); + } // init atoms in use. #define ATOM(name, val) \ @@ -1052,19 +1064,10 @@ on_upgrade(ErlNifEnv *env, static void on_unload(__unused_parm__ ErlNifEnv *env, __unused_parm__ void *priv_data) { - if (G_r_ctx) - { - // @TODO memleak here - MsQuic->RegistrationClose(G_r_ctx->Registration); - G_r_ctx = NULL; - } - if (MsQuic) - { - MsQuicClose(MsQuic); - MsQuic = NULL; - } - // @TODO memleak here - //enif_mutex_destroy(MsQuicLock); + closeLib(env, 0, NULL); + // @TODO reserved for upgrade + // enif_mutex_destroy(GRegLock); + // enif_mutex_destroy(MsQuicLock); } static ERL_NIF_TERM @@ -1078,20 +1081,17 @@ openLib(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) char lttngPath[PATH_MAX] = { 0 }; if (MsQuicLock == NULL) { - return ATOM_OK; + return ERROR_TUPLE_2(ATOM_LIB_UNINITIALIZED); } enif_mutex_lock(MsQuicLock); if (MsQuic) { + // already opened TP_NIF_3(skip, 0, 2); - res = SUCCESS(res); + res = SUCCESS(res); goto exit; } - // @todo external call for static link - CxPlatSystemLoad(); - MsQuicLibraryLoad(); - // // Open a handle to the library and get the API function table. // @@ -1127,19 +1127,31 @@ closeLib(__unused_parm__ ErlNifEnv *env, { if (MsQuicLock == NULL) { - return ATOM_OK; + return ERROR_TUPLE_2(ATOM_LIB_UNINITIALIZED); } enif_mutex_lock(MsQuicLock); if (MsQuic) { TP_NIF_3(do_close, MsQuic, 0); - if (G_r_ctx) + + enif_mutex_lock(GRegLock); + // end of the world + if (G_r_ctx && !G_r_ctx->is_released) { - deregistration(env, argc, argv); + // Make MsQuic debug check pass: + // Zero Registration when closing MsQuic + MsQuic->RegistrationClose(G_r_ctx->Registration); + G_r_ctx->Registration = NULL; + G_r_ctx->is_released = TRUE; + destroy_r_ctx(G_r_ctx); + G_r_ctx = NULL; } + enif_mutex_unlock(GRegLock); + MsQuicClose(MsQuic); MsQuic = NULL; } + enif_mutex_unlock(MsQuicLock); return ATOM_OK; } diff --git a/c_src/quicer_reg.c b/c_src/quicer_reg.c index 6d6f5aaa..55b3531b 100644 --- a/c_src/quicer_reg.c +++ b/c_src/quicer_reg.c @@ -20,6 +20,7 @@ static BOOLEAN parse_reg_conf(ERL_NIF_TERM eprofile, QUIC_REGISTRATION_CONFIG *RegConfig); QuicerRegistrationCTX *G_r_ctx = NULL; +ErlNifMutex *GRegLock = NULL; /* ** For global registration only @@ -33,16 +34,18 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) QUIC_STATUS status; ERL_NIF_TERM res = ATOM_OK; - if (!MsQuic || G_r_ctx) + if (!MsQuic || !GRegLock || G_r_ctx) { return ERROR_TUPLE_2(ATOM_BADARG); } + enif_mutex_lock(GRegLock); if (argc == 1) { eprofile = argv[0]; if (!parse_reg_conf(eprofile, &RegConfig)) { + enif_mutex_unlock(GRegLock); return ERROR_TUPLE_2(ATOM_BADARG); } } @@ -50,6 +53,7 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) QuicerRegistrationCTX *r_ctx = init_r_ctx(); if (!r_ctx) { + enif_mutex_unlock(GRegLock); return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); } @@ -60,14 +64,16 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) goto exit; } - // Keep global registration context - // enif_keep_resource(r_ctx); G_r_ctx = r_ctx; + enif_mutex_unlock(GRegLock); + + // nif owns the global registration + // thus not return to the erlang side return ATOM_OK; exit: - deinit_r_ctx(r_ctx); destroy_r_ctx(r_ctx); + enif_mutex_unlock(GRegLock); return res; } @@ -80,18 +86,19 @@ deregistration(__unused_parm__ ErlNifEnv *env, __unused_parm__ const ERL_NIF_TERM argv[]) { int error_code = 0; - if (!MsQuic) + if (!MsQuic || !GRegLock) { return ERROR_TUPLE_2(ATOM_BADARG); } - if (G_r_ctx) + enif_mutex_lock(GRegLock); + if (G_r_ctx && !G_r_ctx->is_released) { MsQuic->RegistrationShutdown(G_r_ctx->Registration, FALSE, error_code); destroy_r_ctx(G_r_ctx); G_r_ctx = NULL; } - + enif_mutex_unlock(GRegLock); return ATOM_OK; } @@ -137,7 +144,6 @@ new_registration2(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) return SUCCESS(enif_make_resource(env, r_ctx)); exit: - deinit_r_ctx(r_ctx); destroy_r_ctx(r_ctx); return res; } diff --git a/test/quicer_SUITE.erl b/test/quicer_SUITE.erl index 75e68af3..4b986223 100644 --- a/test/quicer_SUITE.erl +++ b/test/quicer_SUITE.erl @@ -33,7 +33,7 @@ end_per_testcase/2]). %% test cases --export([tc_nif_module_load/1 +-export([ tc_nif_module_load/1 , tc_nif_module_unload/1 , tc_nif_module_reload/1 , tc_open_lib_test/1 @@ -219,8 +219,12 @@ end_per_testcase(tc_close_lib_test, _Config) -> quicer:open_lib(); end_per_testcase(tc_lib_registration, _Config) -> quicer:reg_open(); +end_per_testcase(tc_lib_registration_1, _Config) -> + quicer:reg_open(); end_per_testcase(tc_lib_re_registration, _Config) -> quicer:reg_open(); +end_per_testcase(tc_lib_re_registration_neg, _Config) -> + quicer:reg_open(); end_per_testcase(tc_open_listener_neg_1, _Config) -> quicer:open_lib(), quicer:reg_open(); @@ -284,7 +288,7 @@ tc_lib_registration(_Config) -> ok = quicer:reg_close(). tc_lib_registration_1(_Config) -> - ok =quicer:reg_close(), + ok = quicer:reg_close(), {error, badarg} = quicer:reg_open(foo), ok = quicer:reg_open(quic_execution_profile_low_latency), ok = quicer:reg_close(), @@ -819,7 +823,7 @@ dgram_client_recv_loop(Conn, ReceivedOnStream, ReceivedViaDgram) -> receive {quic, <<"pong">>, Conn, Flag} when is_integer(Flag) -> dgram_client_recv_loop(Conn, ReceivedOnStream, true); - {quic, <<"pong">>, _Stream, Flag} -> + {quic, <<"pong">>, _Stream, _Flag} -> dgram_client_recv_loop(Conn, true, ReceivedViaDgram); {quic, dgram_state_changed, Conn, #{dgram_send_enabled := true, dgram_max_len := _Size}} -> dgram_client_recv_loop(Conn, ReceivedOnStream, ReceivedViaDgram); From 36da2aef7f21bf3d0de788d846b2bb0a8738366d Mon Sep 17 00:00:00 2001 From: William Yang Date: Tue, 26 Sep 2023 13:52:13 +0200 Subject: [PATCH 4/6] fix: cacertifle leak in listener neg cases --- c_src/quicer_listener.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index d8e2a386..f590f165 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -276,6 +276,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) if (!parse_cacertfile_option(env, options, &cacertfile)) { // TLS opt error not file content error + free(cacertfile); return ERROR_TUPLE_2(ATOM_CACERTFILE); } @@ -284,6 +285,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) if (!l_ctx) { + free(cacertfile); return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); } @@ -410,6 +412,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) return OK_TUPLE_2(listenHandle); exit: // errors.. + free(cacertfile); free_certificate(&CredConfig); destroy_l_ctx(l_ctx); return ret; From b95070d74624a19f7520ba86ff9128c8ca0c3f58 Mon Sep 17 00:00:00 2001 From: William Yang Date: Tue, 26 Sep 2023 17:58:38 +0200 Subject: [PATCH 5/6] squash: GRegLock switch to pthread mutex --- c_src/quicer_nif.c | 11 +++-------- c_src/quicer_reg.c | 21 +++++++++++---------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index b11482ce..293dced9 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -774,7 +774,7 @@ ERL_NIF_TERM ATOM_QUIC_DATAGRAM_SEND_CANCELED; ATOM(ATOM_UNDEFINED, undefined); extern QuicerRegistrationCTX *G_r_ctx; -extern ErlNifMutex *GRegLock; +extern pthread_mutex_t GRegLock; const QUIC_API_TABLE *MsQuic = NULL; // Mutex for MsQuic @@ -978,11 +978,6 @@ on_load(ErlNifEnv *env, MsQuicLock = enif_mutex_create("msquic_lock"); } - if (!GRegLock) - { - GRegLock = enif_mutex_create("global_reg_lock"); - } - // init atoms in use. #define ATOM(name, val) \ { \ @@ -1134,7 +1129,7 @@ closeLib(__unused_parm__ ErlNifEnv *env, { TP_NIF_3(do_close, MsQuic, 0); - enif_mutex_lock(GRegLock); + pthread_mutex_lock(&GRegLock); // end of the world if (G_r_ctx && !G_r_ctx->is_released) { @@ -1146,7 +1141,7 @@ closeLib(__unused_parm__ ErlNifEnv *env, destroy_r_ctx(G_r_ctx); G_r_ctx = NULL; } - enif_mutex_unlock(GRegLock); + pthread_mutex_unlock(&GRegLock); MsQuicClose(MsQuic); MsQuic = NULL; diff --git a/c_src/quicer_reg.c b/c_src/quicer_reg.c index 55b3531b..91bdf52f 100644 --- a/c_src/quicer_reg.c +++ b/c_src/quicer_reg.c @@ -20,7 +20,7 @@ static BOOLEAN parse_reg_conf(ERL_NIF_TERM eprofile, QUIC_REGISTRATION_CONFIG *RegConfig); QuicerRegistrationCTX *G_r_ctx = NULL; -ErlNifMutex *GRegLock = NULL; +pthread_mutex_t GRegLock = PTHREAD_MUTEX_INITIALIZER; /* ** For global registration only @@ -34,18 +34,19 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) QUIC_STATUS status; ERL_NIF_TERM res = ATOM_OK; - if (!MsQuic || !GRegLock || G_r_ctx) + if (!MsQuic || G_r_ctx) { return ERROR_TUPLE_2(ATOM_BADARG); } - enif_mutex_lock(GRegLock); + pthread_mutex_lock(&GRegLock); + if (argc == 1) { eprofile = argv[0]; if (!parse_reg_conf(eprofile, &RegConfig)) { - enif_mutex_unlock(GRegLock); + pthread_mutex_unlock(&GRegLock); return ERROR_TUPLE_2(ATOM_BADARG); } } @@ -53,7 +54,7 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) QuicerRegistrationCTX *r_ctx = init_r_ctx(); if (!r_ctx) { - enif_mutex_unlock(GRegLock); + pthread_mutex_unlock(&GRegLock); return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); } @@ -65,7 +66,7 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) } G_r_ctx = r_ctx; - enif_mutex_unlock(GRegLock); + pthread_mutex_unlock(&GRegLock); // nif owns the global registration // thus not return to the erlang side @@ -73,7 +74,7 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) exit: destroy_r_ctx(r_ctx); - enif_mutex_unlock(GRegLock); + pthread_mutex_unlock(&GRegLock); return res; } @@ -86,19 +87,19 @@ deregistration(__unused_parm__ ErlNifEnv *env, __unused_parm__ const ERL_NIF_TERM argv[]) { int error_code = 0; - if (!MsQuic || !GRegLock) + if (!MsQuic) { return ERROR_TUPLE_2(ATOM_BADARG); } - enif_mutex_lock(GRegLock); + pthread_mutex_lock(&GRegLock); if (G_r_ctx && !G_r_ctx->is_released) { MsQuic->RegistrationShutdown(G_r_ctx->Registration, FALSE, error_code); destroy_r_ctx(G_r_ctx); G_r_ctx = NULL; } - enif_mutex_unlock(GRegLock); + pthread_mutex_unlock(&GRegLock); return ATOM_OK; } From bb98eaf166e93b1b7592d25b375e55b9b4038569 Mon Sep 17 00:00:00 2001 From: William Yang Date: Tue, 26 Sep 2023 18:11:16 +0200 Subject: [PATCH 6/6] squash: MsQuicLock use pthread_mutex instead --- c_src/quicer_nif.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 293dced9..bb72801c 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -778,7 +778,7 @@ extern pthread_mutex_t GRegLock; const QUIC_API_TABLE *MsQuic = NULL; // Mutex for MsQuic -ErlNifMutex *MsQuicLock = NULL; +pthread_mutex_t MsQuicLock = PTHREAD_MUTEX_INITIALIZER; ErlNifResourceType *ctx_reg_t = NULL; ErlNifResourceType *ctx_listener_t = NULL; @@ -972,12 +972,6 @@ on_load(ErlNifEnv *env, { int ret_val = 0; - // Library initialization, library scope - if (!MsQuicLock) - { - MsQuicLock = enif_mutex_create("msquic_lock"); - } - // init atoms in use. #define ATOM(name, val) \ { \ @@ -1060,9 +1054,6 @@ static void on_unload(__unused_parm__ ErlNifEnv *env, __unused_parm__ void *priv_data) { closeLib(env, 0, NULL); - // @TODO reserved for upgrade - // enif_mutex_destroy(GRegLock); - // enif_mutex_destroy(MsQuicLock); } static ERL_NIF_TERM @@ -1074,11 +1065,8 @@ openLib(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) ERL_NIF_TERM res = ATOM_FALSE; ERL_NIF_TERM lttngLib = argv[0]; char lttngPath[PATH_MAX] = { 0 }; - if (MsQuicLock == NULL) - { - return ERROR_TUPLE_2(ATOM_LIB_UNINITIALIZED); - } - enif_mutex_lock(MsQuicLock); + + pthread_mutex_lock(&MsQuicLock); if (MsQuic) { // already opened @@ -1111,7 +1099,7 @@ openLib(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } exit: - enif_mutex_unlock(MsQuicLock); + pthread_mutex_unlock(&MsQuicLock); return res; } @@ -1120,11 +1108,7 @@ closeLib(__unused_parm__ ErlNifEnv *env, __unused_parm__ int argc, __unused_parm__ const ERL_NIF_TERM argv[]) { - if (MsQuicLock == NULL) - { - return ERROR_TUPLE_2(ATOM_LIB_UNINITIALIZED); - } - enif_mutex_lock(MsQuicLock); + pthread_mutex_lock(&MsQuicLock); if (MsQuic) { TP_NIF_3(do_close, MsQuic, 0); @@ -1147,7 +1131,7 @@ closeLib(__unused_parm__ ErlNifEnv *env, MsQuic = NULL; } - enif_mutex_unlock(MsQuicLock); + pthread_mutex_unlock(&MsQuicLock); return ATOM_OK; }