From 7f065dae406d522c98e3fa3e9da1e9ebecc22d58 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 08:32:16 +0200 Subject: [PATCH 01/10] ci: add asan check --- .github/workflows/asan.yml | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .github/workflows/asan.yml diff --git a/.github/workflows/asan.yml b/.github/workflows/asan.yml new file mode 100644 index 00000000..b4d109be --- /dev/null +++ b/.github/workflows/asan.yml @@ -0,0 +1,34 @@ +name: asan check +on: + workflow_dispatch: + inputs: + ref: + required: false + +jobs: + asan: + runs-on: ubuntu-22.04 + strategy: + fail-fast: true + steps: + - name: checkout + uses: actions/checkout@v3 + with: + ref: ${{ github.event.inputs.ref }} + - name: asan build + run: | + otp_prebuilds=otp-26.2.5.3-ubuntu-22.04.tar.gz + wget https://github.com/qzhuyan/kerl/releases/download/testing/${otp_prebuilds} + tar zxvf ${otp_prebuilds} -C / + ln -s /home/runner/OTP/otp-26.2.5.3/ /home/runner/OTP/default + echo ". /home/runner/OTP/default/activate" >> ~/.bashrc + - name: run sanitizer-check + run: | + . /home/runner/OTP/default/activate + tools/asan/bin/sanitizer-check all + - name: Archive asan logs + uses: actions/upload-artifact@v3 + with: + name: asan_logs + path: asan_logs + retention-days: 7 From 8a29d3e52eb185156484aadfa5431066a3a73c7d Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 11:57:56 +0200 Subject: [PATCH 02/10] test: add debug and valgrind emu checks --- tools/asan/README.md | 2 +- tools/asan/bin/debug-check | 53 +++++++++++++++++++++++++++++++ tools/asan/bin/debug-gdb-core | 10 ++++++ tools/asan/bin/erl-debug | 14 ++++++++ tools/asan/bin/erl-valgrind | 14 ++++++++ tools/asan/bin/valgrind-check | 60 +++++++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 1 deletion(-) create mode 100755 tools/asan/bin/debug-check create mode 100755 tools/asan/bin/debug-gdb-core create mode 100755 tools/asan/bin/erl-debug create mode 100755 tools/asan/bin/erl-valgrind create mode 100755 tools/asan/bin/valgrind-check diff --git a/tools/asan/README.md b/tools/asan/README.md index b91dbffe..216bb579 100644 --- a/tools/asan/README.md +++ b/tools/asan/README.md @@ -2,7 +2,7 @@ ## Prepare -Build otp with debug type: asan +Build otp with emu type: asan ## Run diff --git a/tools/asan/bin/debug-check b/tools/asan/bin/debug-check new file mode 100755 index 00000000..a53693b8 --- /dev/null +++ b/tools/asan/bin/debug-check @@ -0,0 +1,53 @@ +#!/bin/bash +# +# Usage: +# # run all test cases +# sanitizer-check +# +# # run one testcase at a time +# sanitizer-check one_by_one +# +# # run with ct options +# sanitizer-check --case=Case0 --repeat 100 +# +# +lib_dir=$(dirname "$(realpath "$0")") + +# for compiler flag +#export QUICER_USE_SANITIZERS=1 + +# for using asan emulator +export ESCRIPT_EMULATOR="${lib_dir}/erl-debug" + +# Since cerl returns different code:root_dir(), we need to override it here +# Erlang_OTP_ROOT_DIR will be picked up by CMakeLists.txt +export Erlang_OTP_ROOT_DIR=$(dirname $(dirname $(which erl))) + +# Set ERL_TOP for Suppressions list +export ERL_TOP=$(cerl -noshell -eval "io:format(\"~s\", [code:root_dir()])" -s erlang halt) + +#export QUICER_USE_SNK=1 + +REBAR3=$(command -v rebar3) + +if [ $# -eq 1 ]; then + case $1 in + all) + escript "$REBAR3" ct + ;; + one_by_one) + AllTCs=$(erl -pa _build/test/lib/quicer/test/ -noshell \ + -eval 'io:format("~p", [lists:flatten( string:join(lists:map(fun erlang:atom_to_list/1, quicer_SUITE:all()), " ") )]), halt()') + for tc in ${AllTCs}; + do + echo "running tc $tc"; + escript "$REBAR3" do ct --suite=test/quicer_SUITE --case="$tc"; + done + ;; + proper) + escript "$REBAR3" as test proper + ;; + esac +else + escript "$REBAR3" $@ +fi diff --git a/tools/asan/bin/debug-gdb-core b/tools/asan/bin/debug-gdb-core new file mode 100755 index 00000000..90898e2f --- /dev/null +++ b/tools/asan/bin/debug-gdb-core @@ -0,0 +1,10 @@ +#!/bin/bash + +core=$(find . -type f -name "core.*" -printf '%T+ %p\n' | sort -r | head -n 1 | cut -d ' ' -f 2) + +if [ -z "$core" ]; then + echo "No core file found" + exit 1 +fi + +cerl -debug -rcore $core diff --git a/tools/asan/bin/erl-debug b/tools/asan/bin/erl-debug new file mode 100755 index 00000000..00e9b6a6 --- /dev/null +++ b/tools/asan/bin/erl-debug @@ -0,0 +1,14 @@ +#!/bin/bash +# + +die() { + echo "error: $1"; + exit 1; +} + +CERL=`command -v cerl` + +if [ -z "$CERL" ];then + die "cerl is missing, see https://www.erlang.org/doc/installation_guide/install" +fi +$CERL -debug "$@" diff --git a/tools/asan/bin/erl-valgrind b/tools/asan/bin/erl-valgrind new file mode 100755 index 00000000..00d33ad0 --- /dev/null +++ b/tools/asan/bin/erl-valgrind @@ -0,0 +1,14 @@ +#!/bin/bash +# + +die() { + echo "error: $1"; + exit 1; +} + +CERL=`command -v cerl` + +if [ -z "$CERL" ];then + die "cerl is missing, see https://www.erlang.org/doc/installation_guide/install" +fi +$CERL -valgrind "$@" diff --git a/tools/asan/bin/valgrind-check b/tools/asan/bin/valgrind-check new file mode 100755 index 00000000..292c79e5 --- /dev/null +++ b/tools/asan/bin/valgrind-check @@ -0,0 +1,60 @@ +#!/bin/bash +# +# Usage: +# # run all test cases +# sanitizer-check +# +# # run one testcase at a time +# sanitizer-check one_by_one +# +# # run with ct options +# sanitizer-check --case=Case0 --repeat 100 +# +# +lib_dir=$(dirname "$(realpath "$0")") + +# for compiler flag +#export QUICER_USE_SANITIZERS=1 + +# for using asan emulator +export ESCRIPT_EMULATOR="${lib_dir}/erl-valgrind" + +# Since cerl returns different code:root_dir(), we need to override it here +# Erlang_OTP_ROOT_DIR will be picked up by CMakeLists.txt +export Erlang_OTP_ROOT_DIR=$(dirname $(dirname $(which erl))) + +# Set ERL_TOP for Suppressions list +export ERL_TOP=$(cerl -noshell -eval "io:format(\"~s\", [code:root_dir()])" -s erlang halt) + +#export QUICER_USE_SNK=1 + +# For log output +if [ -z "${VALGRIND_LOG_DIR}" ]; then + export VALGRIND_LOG_DIR=${PWD}/valgrind_logs +fi + +mkdir -p "${VALGRIND_LOG_DIR}" + +REBAR3=$(command -v rebar3) + +if [ $# -eq 1 ]; then + case $1 in + all) + escript "$REBAR3" ct + ;; + one_by_one) + AllTCs=$(erl -pa _build/test/lib/quicer/test/ -noshell \ + -eval 'io:format("~p", [lists:flatten( string:join(lists:map(fun erlang:atom_to_list/1, quicer_SUITE:all()), " ") )]), halt()') + for tc in ${AllTCs}; + do + echo "running tc $tc"; + escript "$REBAR3" do ct --suite=test/quicer_SUITE --case="$tc"; + done + ;; + proper) + escript "$REBAR3" as test proper + ;; + esac +else + escript "$REBAR3" $@ +fi From ec219de0a069fa737a3a279766ade0b24974a4db Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 11:59:18 +0200 Subject: [PATCH 03/10] chore: rename tools/asan to tools/run --- .github/workflows/asan.yml | 2 +- tools/{asan => run}/README.md | 0 tools/{asan => run}/bin/debug-check | 6 +++--- tools/{asan => run}/bin/debug-gdb-core | 0 tools/{asan => run}/bin/erl-asan | 0 tools/{asan => run}/bin/erl-debug | 0 tools/{asan => run}/bin/erl-valgrind | 0 tools/{asan => run}/bin/sanitizer-check | 0 tools/{asan => run}/bin/valgrind-check | 8 ++++---- 9 files changed, 8 insertions(+), 8 deletions(-) rename tools/{asan => run}/README.md (100%) rename tools/{asan => run}/bin/debug-check (93%) rename tools/{asan => run}/bin/debug-gdb-core (100%) rename tools/{asan => run}/bin/erl-asan (100%) rename tools/{asan => run}/bin/erl-debug (100%) rename tools/{asan => run}/bin/erl-valgrind (100%) rename tools/{asan => run}/bin/sanitizer-check (100%) rename tools/{asan => run}/bin/valgrind-check (92%) diff --git a/.github/workflows/asan.yml b/.github/workflows/asan.yml index b4d109be..54a8a860 100644 --- a/.github/workflows/asan.yml +++ b/.github/workflows/asan.yml @@ -25,7 +25,7 @@ jobs: - name: run sanitizer-check run: | . /home/runner/OTP/default/activate - tools/asan/bin/sanitizer-check all + tools/run/bin/sanitizer-check all - name: Archive asan logs uses: actions/upload-artifact@v3 with: diff --git a/tools/asan/README.md b/tools/run/README.md similarity index 100% rename from tools/asan/README.md rename to tools/run/README.md diff --git a/tools/asan/bin/debug-check b/tools/run/bin/debug-check similarity index 93% rename from tools/asan/bin/debug-check rename to tools/run/bin/debug-check index a53693b8..1389dcb5 100755 --- a/tools/asan/bin/debug-check +++ b/tools/run/bin/debug-check @@ -2,13 +2,13 @@ # # Usage: # # run all test cases -# sanitizer-check +# debug-check # # # run one testcase at a time -# sanitizer-check one_by_one +# debug-check one_by_one # # # run with ct options -# sanitizer-check --case=Case0 --repeat 100 +# debug-check --case=Case0 --repeat 100 # # lib_dir=$(dirname "$(realpath "$0")") diff --git a/tools/asan/bin/debug-gdb-core b/tools/run/bin/debug-gdb-core similarity index 100% rename from tools/asan/bin/debug-gdb-core rename to tools/run/bin/debug-gdb-core diff --git a/tools/asan/bin/erl-asan b/tools/run/bin/erl-asan similarity index 100% rename from tools/asan/bin/erl-asan rename to tools/run/bin/erl-asan diff --git a/tools/asan/bin/erl-debug b/tools/run/bin/erl-debug similarity index 100% rename from tools/asan/bin/erl-debug rename to tools/run/bin/erl-debug diff --git a/tools/asan/bin/erl-valgrind b/tools/run/bin/erl-valgrind similarity index 100% rename from tools/asan/bin/erl-valgrind rename to tools/run/bin/erl-valgrind diff --git a/tools/asan/bin/sanitizer-check b/tools/run/bin/sanitizer-check similarity index 100% rename from tools/asan/bin/sanitizer-check rename to tools/run/bin/sanitizer-check diff --git a/tools/asan/bin/valgrind-check b/tools/run/bin/valgrind-check similarity index 92% rename from tools/asan/bin/valgrind-check rename to tools/run/bin/valgrind-check index 292c79e5..f9c289d1 100755 --- a/tools/asan/bin/valgrind-check +++ b/tools/run/bin/valgrind-check @@ -2,19 +2,19 @@ # # Usage: # # run all test cases -# sanitizer-check +# valgrind-check # # # run one testcase at a time -# sanitizer-check one_by_one +# valgrind-check one_by_one # # # run with ct options -# sanitizer-check --case=Case0 --repeat 100 +# valgrind-check --case=Case0 --repeat 100 # # lib_dir=$(dirname "$(realpath "$0")") # for compiler flag -#export QUICER_USE_SANITIZERS=1 +export QUICER_USE_SANITIZERS=1 # for using asan emulator export ESCRIPT_EMULATOR="${lib_dir}/erl-valgrind" From 35ef5de302699c0091764c00aa5463830c255b8d Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 21 Oct 2024 14:53:18 +0200 Subject: [PATCH 04/10] update doc --- tools/run/README.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tools/run/README.md b/tools/run/README.md index 216bb579..32ed0ea1 100644 --- a/tools/run/README.md +++ b/tools/run/README.md @@ -2,22 +2,40 @@ ## Prepare -Build otp with emu type: asan +Build otp with different emu types. ## Run +Supported runs are + +- sanitizer-check +- debug-check +- valgrind-check + ### Run all tests +Just take `sanitizer-check` as examples + ``` sh -tools/asan/bin/sanitizer-check all +tools/run/bin/sanitizer-check all ``` ### Run one test in the suite ``` sh -tools/asan/bin/sanitizer-check --suite=SUITE --case=CASE +tools/run/bin/sanitizer-check --suite=SUITE --case=CASE ``` ### Run CT test cases one by one ``` sh -tools/asan/bin/sanitizer-check one_by_one +tools/run/bin/sanitizer-check one_by_one ``` + +## Check coredumps + +Debug emu generates coredump when it isn't happy, +to check the latest core file with gdb use following + +``` sh +tools/run/bin/debug-gdb-core +``` + From b34d9b9e75f82ebf2a2d601d976dd8cfe7519099 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 14:12:36 +0200 Subject: [PATCH 05/10] chore: please debug emu --- c_src/quicer_connection.c | 26 +++++++++++++++++++++----- c_src/quicer_ctx.c | 13 +++++++++++-- c_src/quicer_ctx.h | 3 +++ c_src/quicer_listener.c | 1 + c_src/quicer_nif.c | 25 ++++++++++++++++++++----- c_src/quicer_stream.c | 4 ++++ 6 files changed, 60 insertions(+), 12 deletions(-) diff --git a/c_src/quicer_connection.c b/c_src/quicer_connection.c index ae1b4540..abf1bde1 100644 --- a/c_src/quicer_connection.c +++ b/c_src/quicer_connection.c @@ -841,7 +841,14 @@ async_connect3(ErlNifEnv *env, // Monitor owner before start, so we don't need to race with callbacks // after start the connection - enif_monitor_process(NULL, c_ctx, &c_ctx->owner->Pid, &c_ctx->owner_mon); + // + if (!c_ctx->is_monitored + && 0 + == enif_monitor_process( + NULL, c_ctx, &c_ctx->owner->Pid, &c_ctx->owner_mon)) + { + c_ctx->is_monitored = TRUE; + } // c_ctx->lock should be taken to prevent parallel access from callback as // work trigged by starting of the connection. @@ -1168,7 +1175,11 @@ handle_connection_event_connected(QuicerConnCTX *c_ctx, // A monitor is automatically removed when it triggers or when the // resource is deallocated. enif_mutex_lock(c_ctx->lock); - enif_monitor_process(NULL, c_ctx, acc_pid, &c_ctx->owner_mon); + if ((!c_ctx->is_monitored) + && 0 == enif_monitor_process(NULL, c_ctx, acc_pid, &c_ctx->owner_mon)) + { + c_ctx->is_monitored = TRUE; + } enif_mutex_unlock(c_ctx->lock); ERL_NIF_TERM ConnHandle = enif_make_resource(c_ctx->env, c_ctx); @@ -1392,8 +1403,13 @@ handle_connection_event_peer_stream_started(QuicerConnCTX *c_ctx, = { enif_make_uint(env, Event->PEER_STREAM_STARTED.Flags), ATOM_BOOLEAN(is_orphan) }; - ERL_NIF_TERM report = make_event_with_props( - env, ATOM_NEW_STREAM, s_ctx->eHandle, props_name, props_value, 2); + ERL_NIF_TERM report + = make_event_with_props(env, + ATOM_NEW_STREAM, + enif_make_copy(env, s_ctx->eHandle), + props_name, + props_value, + 2); if (!enif_send(NULL, acc_pid, NULL, report)) { if (is_orphan) @@ -1421,7 +1437,7 @@ handle_connection_event_peer_stream_started(QuicerConnCTX *c_ctx, report = make_event_with_props(env, ATOM_NEW_STREAM, - s_ctx->eHandle, + enif_make_copy(env, s_ctx->eHandle), props_name, props_value, 2); diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index 68fc8c76..43f85280 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -129,7 +129,11 @@ destroy_l_ctx(QuicerListenerCTX *l_ctx) l_ctx->r_ctx = NULL; } l_ctx->config_resource = NULL; - enif_demonitor_process(l_ctx->env, l_ctx, &l_ctx->owner_mon); + if (l_ctx->is_monitored) + { + enif_demonitor_process(l_ctx->env, l_ctx, &l_ctx->owner_mon); + l_ctx->is_monitored = FALSE; + } enif_release_resource(l_ctx); } @@ -212,7 +216,12 @@ destroy_c_ctx(QuicerConnCTX *c_ctx) CxPlatListEntryRemove(&c_ctx->RegistrationLink); enif_mutex_unlock(r_ctx->lock); - enif_demonitor_process(c_ctx->env, c_ctx, &c_ctx->owner_mon); + if (c_ctx->is_monitored) + { + enif_demonitor_process(c_ctx->env, c_ctx, &c_ctx->owner_mon); + c_ctx->is_monitored = FALSE; + } + enif_release_resource(c_ctx); } diff --git a/c_src/quicer_ctx.h b/c_src/quicer_ctx.h index 45204b83..9482ed93 100644 --- a/c_src/quicer_ctx.h +++ b/c_src/quicer_ctx.h @@ -63,6 +63,7 @@ typedef struct QuicerListenerCTX CXPLAT_REF_COUNT ref_count; QUICER_ACCEPTOR_QUEUE *acceptor_queue; ErlNifPid listenerPid; + BOOLEAN is_monitored; ErlNifMonitor owner_mon; ErlNifEnv *env; ErlNifMutex *lock; @@ -92,6 +93,7 @@ typedef struct QuicerConnCTX HQUIC Connection; QUICER_ACCEPTOR_QUEUE *acceptor_queue; ACCEPTOR *owner; + BOOLEAN is_monitored; ErlNifMonitor owner_mon; ErlNifEnv *env; ErlNifMutex *lock; @@ -119,6 +121,7 @@ typedef struct QuicerStreamCTX HQUIC Stream; uint64_t StreamID; ACCEPTOR *owner; + BOOLEAN is_monitored; ErlNifMonitor owner_mon; ErlNifEnv *env; // Immutable env, diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index c13ee2bf..b5e87f7c 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -395,6 +395,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) goto exit; } + l_ctx->is_monitored = TRUE; // Now open listener if (QUIC_FAILED(Status = MsQuic->ListenerOpen( // Listener registration diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 04e8f34d..cd1f27ff 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -1575,7 +1575,11 @@ connection_controlling_process(ErlNifEnv *env, return ERROR_TUPLE_2(ATOM_BADARG); } - enif_demonitor_process(env, c_ctx, &c_ctx->owner_mon); + if (c_ctx->is_monitored) + { + enif_demonitor_process(env, c_ctx, &c_ctx->owner_mon); + c_ctx->is_monitored = FALSE; + } if (0 != enif_monitor_process( @@ -1583,10 +1587,12 @@ connection_controlling_process(ErlNifEnv *env, { // rollback, must success enif_self(env, &c_ctx->owner->Pid); - enif_monitor_process(env, c_ctx, caller, &c_ctx->owner_mon); + CXPLAT_FRE_ASSERT( + 0 == enif_monitor_process(env, c_ctx, caller, &c_ctx->owner_mon)); + c_ctx->is_monitored = TRUE; return ERROR_TUPLE_2(ATOM_OWNER_DEAD); } - + c_ctx->is_monitored = TRUE; TP_NIF_3(exit, (uintptr_t)c_ctx->Connection, (uintptr_t)&c_ctx); return ATOM_OK; } @@ -1610,7 +1616,11 @@ stream_controlling_process(ErlNifEnv *env, return ERROR_TUPLE_2(ATOM_BADARG); } - enif_demonitor_process(env, s_ctx, &s_ctx->owner_mon); + if (s_ctx->is_monitored) + { + enif_demonitor_process(env, s_ctx, &s_ctx->owner_mon); + s_ctx->is_monitored = FALSE; + } if (0 != enif_monitor_process( @@ -1619,9 +1629,14 @@ stream_controlling_process(ErlNifEnv *env, // rollback, must success enif_self(env, &s_ctx->owner->Pid); flush_sig_buffer(env, s_ctx); - enif_monitor_process(env, s_ctx, caller, &s_ctx->owner_mon); + + CXPLAT_FRE_ASSERT( + 0 == enif_monitor_process(env, s_ctx, caller, &s_ctx->owner_mon)); + s_ctx->is_monitored = TRUE; + return ERROR_TUPLE_2(ATOM_OWNER_DEAD); } + s_ctx->is_monitored = TRUE; flush_sig_buffer(env, s_ctx); TP_NIF_3(exit, (uintptr_t)s_ctx->Stream, (uintptr_t)&s_ctx->owner->Pid); return ATOM_OK; diff --git a/c_src/quicer_stream.c b/c_src/quicer_stream.c index 12dc048c..ae7d8746 100644 --- a/c_src/quicer_stream.c +++ b/c_src/quicer_stream.c @@ -378,6 +378,10 @@ async_start_stream2(ErlNifEnv *env, res = ERROR_TUPLE_3(ATOM_STREAM_START_ERROR, ATOM_STATUS(Status)); goto ErrorExit; } + + int mon_res = enif_monitor_process( + env, s_ctx, &s_ctx->owner->Pid, &s_ctx->owner_mon); + CXPLAT_FRE_ASSERT(mon_res == 0); // NOTE: Set is_closed to FALSE (s_ctx->is_closed = FALSE;) // must be done in the worker callback (for // QUICER_STREAM_EVENT_MASK_START_COMPLETE) to avoid race cond. From 231c56921cc8d7d9404c39f67dc251600feceba9 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 14:54:36 +0200 Subject: [PATCH 06/10] test: fix flaky test due to timings --- test/quicer_connection_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/quicer_connection_SUITE.erl b/test/quicer_connection_SUITE.erl index 9c8b65d4..5c70448e 100644 --- a/test/quicer_connection_SUITE.erl +++ b/test/quicer_connection_SUITE.erl @@ -293,7 +293,7 @@ tc_conn_timeout(Config) -> TOut = 10, {SPid, Ref} = spawn_monitor( fun() -> - simple_slow_conn_server(Owner, Config, Port, TOut * 2) + simple_slow_conn_server(Owner, Config, Port, TOut * 5) end ), receive From d8804b7c1e34a489878a4aaa3b74138cb4fc2465 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 15:37:42 +0200 Subject: [PATCH 07/10] fix(client): avoid double free config_resource --- c_src/quicer_connection.c | 1 + 1 file changed, 1 insertion(+) diff --git a/c_src/quicer_connection.c b/c_src/quicer_connection.c index abf1bde1..13630c93 100644 --- a/c_src/quicer_connection.c +++ b/c_src/quicer_connection.c @@ -870,6 +870,7 @@ async_connect3(ErlNifEnv *env, if (c_ctx->config_resource) { destroy_config_ctx(c_ctx->config_resource); + c_ctx->config_resource = NULL; } res = ERROR_TUPLE_2(ATOM_CONN_START_ERROR); From a89ba62b3247b33a855dc3b1205c233bfc3b6ce3 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 17:36:56 +0200 Subject: [PATCH 08/10] test: fix flaky --- test/example_client_connection.erl | 2 +- test/example_server_stream.erl | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/example_client_connection.erl b/test/example_client_connection.erl index 2fd90cd9..75b10591 100644 --- a/test/example_client_connection.erl +++ b/test/example_client_connection.erl @@ -165,4 +165,4 @@ handle_info({quic, Sig, Stream, _} = Msg, #{streams := Streams} = S) when false -> error(fixme) end, - {ok, S}. + {ok, NewS}. diff --git a/test/example_server_stream.erl b/test/example_server_stream.erl index af00f9cb..a7040c9c 100644 --- a/test/example_server_stream.erl +++ b/test/example_server_stream.erl @@ -137,10 +137,11 @@ handle_stream_data( case PeerStream of undefined -> case - quicer_local_stream:start_link( + quicer_local_stream:start( ?MODULE, Conn, - [{open_flag, ?QUIC_STREAM_OPEN_FLAG_UNIDIRECTIONAL}] + [{open_flag, ?QUIC_STREAM_OPEN_FLAG_UNIDIRECTIONAL}], + [] ) of {ok, StreamProc} -> From fdc2350589215e177b545214a48259e22ead4fd7 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 20 Oct 2024 19:58:09 +0200 Subject: [PATCH 09/10] fix(tp): for debug emu runs --- c_src/quicer_nif.c | 4 +++- c_src/quicer_tp.c | 17 ++++++++++++----- tools/run/bin/debug-check | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index cd1f27ff..de6ff29e 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -1140,6 +1140,9 @@ on_load(ErlNifEnv *env, int ret_val = 0; unsigned load_vsn = 0; + init_atoms(env); + + // TP must run after init_atoms as atoms are used in TP TP_NIF_3(start, &MsQuic, 0); if (!enif_get_uint(env, loadinfo, &load_vsn)) { @@ -1154,7 +1157,6 @@ on_load(ErlNifEnv *env, return 1; // any value except 0 is error } - init_atoms(env); open_resources(env); TP_NIF_3(end, &MsQuic, 0); diff --git a/c_src/quicer_tp.c b/c_src/quicer_tp.c index 07324600..b2874e55 100644 --- a/c_src/quicer_tp.c +++ b/c_src/quicer_tp.c @@ -4,6 +4,9 @@ extern uint64_t CxPlatTimeUs64(void); +// help macro to copy atom to env for debug emulator assertions +#define ATOM_IN_ENV(X) enif_make_copy(env, ATOM_##X) + void tp_snk(ErlNifEnv *env, const char *ctx, @@ -17,11 +20,13 @@ tp_snk(ErlNifEnv *env, { ERL_NIF_TERM snk_event; ERL_NIF_TERM snk_event_key_array[7] - = { ATOM_SNK_KIND, ATOM_CONTEXT, ATOM_FUNCTION, ATOM_TAG, - ATOM_RESOURCE_ID, ATOM_MARK, ATOM_SNK_META }; + = { ATOM_IN_ENV(SNK_KIND), ATOM_IN_ENV(CONTEXT), + ATOM_IN_ENV(FUNCTION), ATOM_IN_ENV(TAG), + ATOM_IN_ENV(RESOURCE_ID), ATOM_IN_ENV(MARK), + ATOM_IN_ENV(SNK_META) }; ERL_NIF_TERM snk_evt_meta; - ERL_NIF_TERM snk_evt_meta_key_array[1] = { ATOM_TIME }; + ERL_NIF_TERM snk_evt_meta_key_array[1] = { ATOM_IN_ENV(TIME) }; ERL_NIF_TERM snk_evt_meta_val_array[1] = { enif_make_uint64(env, CxPlatTimeUs64()) }; @@ -33,7 +38,7 @@ tp_snk(ErlNifEnv *env, &snk_evt_meta); ERL_NIF_TERM snk_event_val_array[7] = { - ATOM_DEBUG, // snk_kind + ATOM_IN_ENV(DEBUG), // snk_kind enif_make_string(env, ctx, ERL_NIF_LATIN1), // context enif_make_string(env, fun, ERL_NIF_LATIN1), // fun enif_make_string(env, tag, ERL_NIF_LATIN1), // tag @@ -46,7 +51,9 @@ tp_snk(ErlNifEnv *env, env, snk_event_key_array, snk_event_val_array, 7, &snk_event); ERL_NIF_TERM report = enif_make_tuple2( - env, ATOM_GEN_CAST, enif_make_tuple2(env, ATOM_TRACE, snk_event)); + env, + ATOM_IN_ENV(GEN_CAST), + enif_make_tuple2(env, ATOM_IN_ENV(TRACE), snk_event)); enif_send(NULL, &pid, NULL, report); } } diff --git a/tools/run/bin/debug-check b/tools/run/bin/debug-check index 1389dcb5..2f1a277a 100755 --- a/tools/run/bin/debug-check +++ b/tools/run/bin/debug-check @@ -26,7 +26,7 @@ export Erlang_OTP_ROOT_DIR=$(dirname $(dirname $(which erl))) # Set ERL_TOP for Suppressions list export ERL_TOP=$(cerl -noshell -eval "io:format(\"~s\", [code:root_dir()])" -s erlang halt) -#export QUICER_USE_SNK=1 +export QUICER_USE_SNK=1 REBAR3=$(command -v rebar3) From c0815e2b817d23b7ec177dd6b9611aa78c5c76e0 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 21 Oct 2024 13:13:36 +0200 Subject: [PATCH 10/10] fix: make prop tests stable --- test/example_client_connection.erl | 31 ++++++++++++++++-------------- test/example_client_stream.erl | 13 ++++++++++--- test/example_server_stream.erl | 2 +- test/prop_stateful_server_conn.erl | 2 ++ 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/test/example_client_connection.erl b/test/example_client_connection.erl index 75b10591..7cb93ff4 100644 --- a/test/example_client_connection.erl +++ b/test/example_client_connection.erl @@ -106,7 +106,7 @@ new_stream( } = CBState ) -> %% Spawn new stream - case quicer_remote_stream:start_link(example_server_stream, Stream, Conn, SOpts, Flags) of + case quicer_remote_stream:start_link(example_client_stream, Stream, Conn, SOpts, Flags) of {ok, StreamOwner} -> case quicer:handoff_stream(Stream, StreamOwner) of ok -> @@ -116,7 +116,8 @@ new_stream( {ok, CBState#{streams := [{E, Stream} | Streams]}} end; Other -> - Other + ct:pal("Start accepting remote stream error ~p", [Other]), + {ok, CBState#{streams := [{start_error, Stream} | Streams]}} end. dgram_state_changed(_Conn, _Flags, S) -> @@ -152,17 +153,19 @@ peer_needs_streams(C, bidi_streams, S) -> handle_info({'EXIT', _Pid, _Reason}, State) -> {ok, State}; handle_info({quic, Sig, Stream, _} = Msg, #{streams := Streams} = S) when - %% @FIXME, not desired behavior. - %% Casued by inflight quic Msg during stream handoff Sig == peer_send_shutdown orelse Sig == stream_closed -> - {OwnerPid, Stream} = lists:keyfind(Stream, 2, Streams), - NewS = - case OwnerPid == owner_down orelse OwnerPid == closed of - true -> - quicer:async_shutdown_stream(Stream), - S#{streams := lists:keydelete(Stream, 2, Streams)}; - false -> - error(fixme) - end, - {ok, NewS}. + case lists:keyfind(Stream, 2, Streams) of + {Reason, Stream} when + Reason =:= owner_down orelse + Reason =:= closed orelse + Reason =:= start_error + -> + _ = quicer:async_shutdown_stream(Stream), + {ok, S#{streams := lists:keydelete(Stream, 2, Streams)}}; + {OwnerPid, Stream} when is_pid(OwnerPid) -> + {error, {fixme, bug_handoff_fail}}; + false -> + %% garbage signals from already dead stream (such like crashed owner) + {ok, S} + end. diff --git a/test/example_client_stream.erl b/test/example_client_stream.erl index 91c22ebe..3323eb9e 100644 --- a/test/example_client_stream.erl +++ b/test/example_client_stream.erl @@ -39,9 +39,16 @@ -include("quicer.hrl"). -include_lib("snabbkaffe/include/snabbkaffe.hrl"). -init_handoff(_Stream, _StreamOpts, _Conn, _Flags) -> - %% stream owner already set while starts. - {stop, not_impl, #{}}. +init_handoff(Stream, _StreamOpts, Conn, #{flags := Flags}) -> + InitState = #{ + stream => Stream, + conn => Conn, + peer_stream => undefined, + is_local => false, + is_unidir => quicer:is_unidirectional(Flags) + }, + ct:pal("init_handoff ~p", [{InitState, _StreamOpts}]), + {ok, InitState}. post_handoff(Stream, _PostData, State) -> ok = quicer:setopt(Stream, active, true), diff --git a/test/example_server_stream.erl b/test/example_server_stream.erl index a7040c9c..11b5e6be 100644 --- a/test/example_server_stream.erl +++ b/test/example_server_stream.erl @@ -48,7 +48,7 @@ init_handoff(Stream, _StreamOpts, Conn, #{flags := Flags}) -> is_local => false, is_unidir => quicer:is_unidirectional(Flags) }, - % ct:pal("init_handoff ~p", [{InitState, _StreamOpts}]), + ct:pal("init_handoff ~p", [{InitState, _StreamOpts}]), {ok, InitState}. post_handoff(Stream, _PostData, State) -> diff --git a/test/prop_stateful_server_conn.erl b/test/prop_stateful_server_conn.erl index 8b52ee49..04e6346b 100644 --- a/test/prop_stateful_server_conn.erl +++ b/test/prop_stateful_server_conn.erl @@ -288,6 +288,8 @@ next_state(State, Res, Call) -> do_next_state(#{state := accepted} = State, {error, _}, {call, quicer, handshake, _Args}) -> State; +do_next_state(#{state := _} = State, {error, closed}, {call, quicer, _, _Args}) -> + State#{state := closed}; do_next_state(#{state := accepted} = State, _Res, {call, quicer, handshake, _Args}) -> State#{state := connected}; do_next_state(#{state := accepted} = State, _Res, {call, quicer, close_connection, _Args}) ->