From 943b315e5d050d54caf7751b6e1881052ed60de8 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Fri, 12 Jul 2024 21:37:31 -0700 Subject: [PATCH 01/12] [TEST+CI only] cleanup of sanitizeThread --- .github/workflows/build-test.yml | 20 +++++++++++++++++++- .github/workflows/on-pr-debug.yml | 14 ++++++++++++++ .github/workflows/on-push-release.yml | 23 +++++++++++++++++++++++ test/test.c | 21 ++++++++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 9a1bf957c..144925cbf 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -10,6 +10,9 @@ on: coverage: type: string default: "OFF" + dev_mode: + type: string + default: "OFF" lib_msg_delivery: type: string default: "OFF" @@ -40,6 +43,12 @@ on: type: string description: "Ubuntu version to use, e.g. '20.04'" default: "latest" + verbose_test_output: + type: string + default: "OFF" + verbose_make_output: + type: string + default: "ON" secrets: CODECOV_TOKEN: description: "Codecov repo token" @@ -83,7 +92,16 @@ jobs: if [[ "${{ inputs.coverage }}" == "ON" ]]; then flags="$flags -DNATS_COVERAGE=ON" fi + if [[ "${{ inputs.dev_mode }}" == "ON" ]]; then + flags="$flags -DDEV_MODE=ON" + fi + if [[ "${{ inputs.verbose_make_output }}" == "ON" ]]; then + flags="$flags -DCMAKE_VERBOSE_MAKEFILE=ON" + fi echo "flags=$flags" >> $GITHUB_OUTPUT + if [[ "${{ inputs.verbose_test_output }}" == "ON" || "${{ inputs.sanitize }}" == "thread" ]]; then + echo "VVFLAG=-VV" >> $GITHUB_ENV + fi - id: nats-vars name: Configure NATS_ environment variables @@ -158,7 +176,7 @@ jobs: export PATH=../deps/nats-server:../deps/nats-streaming-server:$PATH export NATS_TEST_SERVER_VERSION="$(nats-server -v)" flags="" - ctest --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }} 2>&1 | tee /tmp/out.txt + ctest $VVFLAG --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }} 2>&1 | tee /tmp/out.txt if [[ $(grep -q 'ThreadSanitizer: ' /tmp/out.txt; echo $?) == 0 ]]; then echo "!!! ThreadSanitizer detected WARNING(s) !!!" exit 1 diff --git a/.github/workflows/on-pr-debug.yml b/.github/workflows/on-pr-debug.yml index eccc0f9f2..de6211dde 100644 --- a/.github/workflows/on-pr-debug.yml +++ b/.github/workflows/on-pr-debug.yml @@ -28,6 +28,16 @@ jobs: secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + dev-mode: + name: "DEV_MODE" + uses: ./.github/workflows/build-test.yml + with: + dev_mode: ON + server_version: main + type: Debug + verbose_test_output: ON + verbose_make_output: ON + sanitize-addr: name: "Sanitize address" uses: ./.github/workflows/build-test.yml @@ -40,6 +50,7 @@ jobs: name: "Sanitize address (lib_msg_delivery)" uses: ./.github/workflows/build-test.yml with: + type: Debug sanitize: address server_version: main lib_msg_delivery: ON @@ -48,6 +59,7 @@ jobs: name: "Sanitize address (lib_write_deadline)" uses: ./.github/workflows/build-test.yml with: + type: Debug sanitize: address server_version: main lib_write_deadline: ON @@ -56,8 +68,10 @@ jobs: name: "Sanitize thread" uses: ./.github/workflows/build-test.yml with: + type: Debug sanitize: thread server_version: main + repeat: 10 Windows: name: "Windows" diff --git a/.github/workflows/on-push-release.yml b/.github/workflows/on-push-release.yml index d47c20308..50513575c 100644 --- a/.github/workflows/on-push-release.yml +++ b/.github/workflows/on-push-release.yml @@ -22,3 +22,26 @@ jobs: server_version: main ubuntu_version: ${{ matrix.ubuntu_version }} compiler: ${{ matrix.compiler }} + + dev-mode: + name: "DEV_MODE" + uses: ./.github/workflows/build-test.yml + with: + dev_mode: ON + server_version: main + verbose_test_output: ON + verbose_make_output: ON + + sanitize-addr: + name: "Sanitize address" + uses: ./.github/workflows/build-test.yml + with: + sanitize: address + server_version: main + + san-thread: + name: "Sanitize thread" + uses: ./.github/workflows/build-test.yml + with: + sanitize: thread + server_version: main diff --git a/test/test.c b/test/test.c index a2c898038..a2e377737 100644 --- a/test/test.c +++ b/test/test.c @@ -5477,6 +5477,7 @@ test_natsSrvVersionAtLeast(void) { s = NATS_ERR; } + natsConn_Unlock(nc); } } testCond(s == NATS_OK); @@ -13337,7 +13338,10 @@ test_ClientAutoUnsubAndReconnect(void) nats_Sleep(10); test("Received no more than max: "); - testCond((s == NATS_OK) && (arg.sum == 10)); + natsMutex_Lock(arg.m); + int sum = arg.sum; + natsMutex_Unlock(arg.m); + testCond((s == NATS_OK) && (sum == 10)); natsSubscription_Destroy(sub); natsConnection_Destroy(nc); @@ -13390,6 +13394,7 @@ test_AutoUnsubNoUnsubOnDestroy(void) natsMutex_Lock(arg.m); while ((s != NATS_TIMEOUT) && !arg.done) s = natsCondition_TimedWait(arg.c, arg.m, 2000); + natsMutex_Unlock(arg.m); testCond(s == NATS_OK); natsConnection_Destroy(nc); @@ -20243,6 +20248,15 @@ test_ForcedReconnect(void) natsMsg *msg = NULL; natsPid pid = NATS_INVALID_PID; +#ifdef __SANITIZE_THREAD__ + // threadSanitizer complains that we close the fd when a read may be in + // progress. Since it appears to work as desired, skipping the test. + + test("Skipping test_ForcedReconnect test because it does not work with thread sanitizer\n"); + testCond(true); + return; +#endif + s = _createDefaultThreadArgsForCbTests(&arg); if (s != NATS_OK) FAIL("unable to setup test"); @@ -20800,6 +20814,7 @@ test_EventLoop(void) natsMutex_Lock(arg.m); if (arg.attached != 2 || !arg.detached) s = NATS_ERR; + natsMutex_Unlock(arg.m); testCond(s == NATS_OK); natsSubscription_Destroy(sub); @@ -29485,6 +29500,9 @@ _jsOrderedErrHandler(natsConnection *nc, natsSubscription *subscription, natsSta { struct threadArg *args = (struct threadArg*) closure; + if (err != NATS_MISSED_HEARTBEAT) + return; + natsMutex_Lock(args->m); args->status = err; natsCondition_Signal(args->c); @@ -29824,6 +29842,7 @@ test_JetStreamOrderedConsSrvRestart(void) natsMutex_Lock(args.m); while ((s != NATS_TIMEOUT) && !args.reconnected) s = natsCondition_TimedWait(args.c, args.m, 2000); + natsMutex_Unlock(args.m); testCond(s == NATS_OK); test("Send 1 message: "); From eb9412cfc35b7cf33432115288541cd4c3a52b0b Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Fri, 12 Jul 2024 21:57:02 -0700 Subject: [PATCH 02/12] Removed repeat 10 --- .github/workflows/on-pr-debug.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/on-pr-debug.yml b/.github/workflows/on-pr-debug.yml index de6211dde..3691d6946 100644 --- a/.github/workflows/on-pr-debug.yml +++ b/.github/workflows/on-pr-debug.yml @@ -71,7 +71,6 @@ jobs: type: Debug sanitize: thread server_version: main - repeat: 10 Windows: name: "Windows" From b1a6e9ce013c9434ea0eb7a1f4673bd668ecca17 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Sat, 13 Jul 2024 11:52:42 -0700 Subject: [PATCH 03/12] use TSAN_OPTIONS --- .github/workflows/build-test.yml | 9 +-------- test/CMakeLists.txt | 6 +++++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 144925cbf..31a340b78 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -99,9 +99,6 @@ jobs: flags="$flags -DCMAKE_VERBOSE_MAKEFILE=ON" fi echo "flags=$flags" >> $GITHUB_OUTPUT - if [[ "${{ inputs.verbose_test_output }}" == "ON" || "${{ inputs.sanitize }}" == "thread" ]]; then - echo "VVFLAG=-VV" >> $GITHUB_ENV - fi - id: nats-vars name: Configure NATS_ environment variables @@ -176,11 +173,7 @@ jobs: export PATH=../deps/nats-server:../deps/nats-streaming-server:$PATH export NATS_TEST_SERVER_VERSION="$(nats-server -v)" flags="" - ctest $VVFLAG --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }} 2>&1 | tee /tmp/out.txt - if [[ $(grep -q 'ThreadSanitizer: ' /tmp/out.txt; echo $?) == 0 ]]; then - echo "!!! ThreadSanitizer detected WARNING(s) !!!" - exit 1 - fi + ctest --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }} 2>&1 | tee /tmp/out.txt - name: Upload coverage reports to Codecov if: inputs.coverage == 'ON' diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 58a4aa14f..7e3cc0fde 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -27,6 +27,7 @@ add_executable(testsuite test.c) # Link statically with the library target_link_libraries(testsuite nats_static ${NATS_EXTRA_LIB}) + # Set the test index to 0 set(testIndex 0) @@ -35,7 +36,6 @@ file(STRINGS list.txt listOfTestNames) # For each test name foreach(name ${listOfTestNames}) - # Create a test and pass the index (start and end are the same) # to the testsuite executable add_test(NAME Test_${name} @@ -45,6 +45,10 @@ foreach(name ${listOfTestNames}) # Make sure the test passes set_tests_properties(Test_${name} PROPERTIES PASS_REGULAR_EXPRESSION "ALL PASSED") + # Set TSAN_OPTIONS for the test + set_tests_properties(Test_${name} PROPERTIES + ENVIRONMENT "TSAN_OPTIONS=detect_deadlocks=1:second_deadlock_stack=1:verbosity=2:halt_on_error=1:report_signal_unsafe=1") + # Bump the test index number math(EXPR testIndex "${testIndex}+1") endforeach() From 7723d1ad8281675deeae5ca13aea31de43b8dacc Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Sat, 13 Jul 2024 12:29:08 -0700 Subject: [PATCH 04/12] PR feedback: use natsSock_Shutdown in natsConnection_Reconnect --- src/conn.c | 2 +- test/test.c | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/conn.c b/src/conn.c index 1b405c852..3287965e3 100644 --- a/src/conn.c +++ b/src/conn.c @@ -3435,7 +3435,7 @@ natsConnection_Reconnect(natsConnection *nc) return nats_setDefaultError(NATS_CONNECTION_CLOSED); } - natsSock_Close(nc->sockCtx.fd); + natsSock_Shutdown(nc->sockCtx.fd); natsConn_Unlock(nc); return NATS_OK; diff --git a/test/test.c b/test/test.c index a2e377737..27b441264 100644 --- a/test/test.c +++ b/test/test.c @@ -20248,15 +20248,6 @@ test_ForcedReconnect(void) natsMsg *msg = NULL; natsPid pid = NATS_INVALID_PID; -#ifdef __SANITIZE_THREAD__ - // threadSanitizer complains that we close the fd when a read may be in - // progress. Since it appears to work as desired, skipping the test. - - test("Skipping test_ForcedReconnect test because it does not work with thread sanitizer\n"); - testCond(true); - return; -#endif - s = _createDefaultThreadArgsForCbTests(&arg); if (s != NATS_OK) FAIL("unable to setup test"); @@ -20266,6 +20257,7 @@ test_ForcedReconnect(void) CHECK_SERVER_STARTED(pid); IFOK(s, natsOptions_Create(&opts)); IFOK(s, natsOptions_SetReconnectedCB(opts, _reconnectedCb, &arg)); + IFOK(s, natsOptions_SetReconnectWait(opts, 100)); IFOK(s, natsConnection_Connect(&nc, opts)); IFOK(s, natsConnection_SubscribeSync(&sub, nc, "foo")); testCond(s == NATS_OK); From 23e8b68a9580a4fd951a047c735699729c60e163 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 01:53:14 -0700 Subject: [PATCH 05/12] PR feedback --- .github/workflows/build-test.yml | 2 +- .travis.yml | 24 ++++++++++++------------ test/CMakeLists.txt | 4 +++- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 31a340b78..18e18c08d 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -173,7 +173,7 @@ jobs: export PATH=../deps/nats-server:../deps/nats-streaming-server:$PATH export NATS_TEST_SERVER_VERSION="$(nats-server -v)" flags="" - ctest --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }} 2>&1 | tee /tmp/out.txt + ctest --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }} - name: Upload coverage reports to Codecov if: inputs.coverage == 'ON' diff --git a/.travis.yml b/.travis.yml index 6ab9ae1ae..df8676b18 100644 --- a/.travis.yml +++ b/.travis.yml @@ -122,18 +122,18 @@ jobs: - MATRIX_EVAL="CC=gcc-9" - NATS_DEFAULT_LIB_WRITE_DEADLINE=2000 BUILD_OPT="-DNATS_BUILD_ARCH=64 -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-fsanitize=address" NATS_TEST_VALGRIND=yes DO_COVERAGE="no" - # - name: "gcc-9 - Default - sanitize thread" - # compiler: gcc - # addons: - # apt: - # sources: - # - ubuntu-toolchain-r-test - # - sourceline: ppa:ubuntu-toolchain-r/test - # packages: - # - g++-9 - # env: - # - MATRIX_EVAL="CC=gcc-9" - # - BUILD_OPT="-DNATS_BUILD_ARCH=64 -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-fsanitize=thread" NATS_TEST_VALGRIND=yes DO_COVERAGE="no" + - name: "gcc-9 - Default - sanitize thread" + compiler: gcc + addons: + apt: + sources: + - ubuntu-toolchain-r-test + - sourceline: ppa:ubuntu-toolchain-r/test + packages: + - g++-9 + env: + - MATRIX_EVAL="CC=gcc-9" + - BUILD_OPT="-DNATS_BUILD_ARCH=64 -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-fsanitize=thread" NATS_TEST_VALGRIND=yes DO_COVERAGE="no" - name: "clang-8 - TLS OFF" compiler: clang diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7e3cc0fde..85d0f0e30 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -46,8 +46,10 @@ foreach(name ${listOfTestNames}) set_tests_properties(Test_${name} PROPERTIES PASS_REGULAR_EXPRESSION "ALL PASSED") # Set TSAN_OPTIONS for the test - set_tests_properties(Test_${name} PROPERTIES + if(NATS_SANITIZE) + set_tests_properties(Test_${name} PROPERTIES ENVIRONMENT "TSAN_OPTIONS=detect_deadlocks=1:second_deadlock_stack=1:verbosity=2:halt_on_error=1:report_signal_unsafe=1") + endif(NATS_SANITIZE) # Bump the test index number math(EXPR testIndex "${testIndex}+1") From 41546673907f3a9f229f81b08329850d42302053 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 02:09:41 -0700 Subject: [PATCH 06/12] disabled sanitize with gcc on travis again --- .github/workflows/on-pr-debug.yml | 33 +++++++++++-------------------- .travis.yml | 24 +++++++++++----------- 2 files changed, 24 insertions(+), 33 deletions(-) diff --git a/.github/workflows/on-pr-debug.yml b/.github/workflows/on-pr-debug.yml index 3691d6946..2f1c71e58 100644 --- a/.github/workflows/on-pr-debug.yml +++ b/.github/workflows/on-pr-debug.yml @@ -38,24 +38,23 @@ jobs: verbose_test_output: ON verbose_make_output: ON - sanitize-addr: - name: "Sanitize address" + sanitize: + name: "Sanitize" + strategy: + fail-fast: false + matrix: + compiler: [gcc, clang] + sanitize: [address, thread] + pooled_delivery: [ON, OFF] uses: ./.github/workflows/build-test.yml with: - sanitize: address server_version: main type: Debug + compiler: ${{ matrix.compiler }} + sanitize: ${{ matrix.sanitize }} + lib_msg_delivery: ${{ matrix.pooled_delivery }} - sanitize-addr-lib-msg-delivery: - name: "Sanitize address (lib_msg_delivery)" - uses: ./.github/workflows/build-test.yml - with: - type: Debug - sanitize: address - server_version: main - lib_msg_delivery: ON - - san-addr: + san-addr-deadline: name: "Sanitize address (lib_write_deadline)" uses: ./.github/workflows/build-test.yml with: @@ -64,14 +63,6 @@ jobs: server_version: main lib_write_deadline: ON - san-thread: - name: "Sanitize thread" - uses: ./.github/workflows/build-test.yml - with: - type: Debug - sanitize: thread - server_version: main - Windows: name: "Windows" runs-on: windows-latest diff --git a/.travis.yml b/.travis.yml index df8676b18..6ab9ae1ae 100644 --- a/.travis.yml +++ b/.travis.yml @@ -122,18 +122,18 @@ jobs: - MATRIX_EVAL="CC=gcc-9" - NATS_DEFAULT_LIB_WRITE_DEADLINE=2000 BUILD_OPT="-DNATS_BUILD_ARCH=64 -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-fsanitize=address" NATS_TEST_VALGRIND=yes DO_COVERAGE="no" - - name: "gcc-9 - Default - sanitize thread" - compiler: gcc - addons: - apt: - sources: - - ubuntu-toolchain-r-test - - sourceline: ppa:ubuntu-toolchain-r/test - packages: - - g++-9 - env: - - MATRIX_EVAL="CC=gcc-9" - - BUILD_OPT="-DNATS_BUILD_ARCH=64 -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-fsanitize=thread" NATS_TEST_VALGRIND=yes DO_COVERAGE="no" + # - name: "gcc-9 - Default - sanitize thread" + # compiler: gcc + # addons: + # apt: + # sources: + # - ubuntu-toolchain-r-test + # - sourceline: ppa:ubuntu-toolchain-r/test + # packages: + # - g++-9 + # env: + # - MATRIX_EVAL="CC=gcc-9" + # - BUILD_OPT="-DNATS_BUILD_ARCH=64 -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-fsanitize=thread" NATS_TEST_VALGRIND=yes DO_COVERAGE="no" - name: "clang-8 - TLS OFF" compiler: clang From 9966b923ab6e56e47c69c8aef4f08a5f8c05883c Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 02:28:06 -0700 Subject: [PATCH 07/12] Fix data race in _resendSubscriptions: add a delivery worker lock --- src/conn.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/conn.c b/src/conn.c index 3287965e3..7cfe9beb1 100644 --- a/src/conn.c +++ b/src/conn.c @@ -1137,17 +1137,29 @@ _resendSubscriptions(natsConnection *nc) adjustedMax = 0; natsSub_Lock(sub); + if (sub->libDlvWorker != NULL) + { + natsMutex_Lock(sub->libDlvWorker->lock); + } // If JS ordered consumer, trigger a reset. Don't check the error // condition here. If there is a failure, it will be retried // at the next HB interval. if ((sub->jsi != NULL) && (sub->jsi->ordered)) { jsSub_resetOrderedConsumer(sub, sub->jsi->sseq+1); + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); continue; } if (natsSub_drainStarted(sub)) { + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); continue; } @@ -1160,6 +1172,10 @@ _resendSubscriptions(natsConnection *nc) // messages have reached the max, if so, unsubscribe. if (adjustedMax == 0) { + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); s = natsConn_sendUnsubProto(nc, sub->sid, 0); continue; @@ -1172,6 +1188,10 @@ _resendSubscriptions(natsConnection *nc) // Hold the lock up to that point so we are sure not to resend // any SUB/UNSUB for a subscription that is in draining mode. + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); } From f54999cdc45a3fb2e77d4e32494d38dec6324a93 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 02:31:09 -0700 Subject: [PATCH 08/12] removed a blank line --- test/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 85d0f0e30..bff993454 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -27,7 +27,6 @@ add_executable(testsuite test.c) # Link statically with the library target_link_libraries(testsuite nats_static ${NATS_EXTRA_LIB}) - # Set the test index to 0 set(testIndex 0) From 48461963add8a4bc81c14e674129faa8ed94935d Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 02:34:25 -0700 Subject: [PATCH 09/12] removed TSAN extra verbosity --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index bff993454..c15ccfc62 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -47,7 +47,7 @@ foreach(name ${listOfTestNames}) # Set TSAN_OPTIONS for the test if(NATS_SANITIZE) set_tests_properties(Test_${name} PROPERTIES - ENVIRONMENT "TSAN_OPTIONS=detect_deadlocks=1:second_deadlock_stack=1:verbosity=2:halt_on_error=1:report_signal_unsafe=1") + ENVIRONMENT "TSAN_OPTIONS=detect_deadlocks=1:second_deadlock_stack=1:halt_on_error=1:report_signal_unsafe=1") endif(NATS_SANITIZE) # Bump the test index number From ee40b6844b5f2ef216622edfa7193da1ee87252d Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 02:40:29 -0700 Subject: [PATCH 10/12] fixed another race, in JetStreamSubscribeIdleHeartbeat --- src/js.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/js.c b/src/js.c index e31dd2f0a..3d3e4bb18 100644 --- a/src/js.c +++ b/src/js.c @@ -2045,6 +2045,10 @@ _hbTimerFired(natsTimer *timer, void* closure) natsStatus s = NATS_OK; natsSub_Lock(sub); + if (sub->libDlvWorker != NULL) + { + natsMutex_Lock(sub->libDlvWorker->lock); + } alert = !jsi->active; oc = jsi->ordered; jsi->active = false; @@ -2062,10 +2066,18 @@ _hbTimerFired(natsTimer *timer, void* closure) natsCondition_Signal(sub->cond); natsTimer_Stop(timer); } + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); return; } nc = sub->conn; + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); if (!alert) @@ -2075,12 +2087,20 @@ _hbTimerFired(natsTimer *timer, void* closure) if (oc) { natsSub_Lock(sub); + if (sub->libDlvWorker != NULL) + { + natsMutex_Lock(sub->libDlvWorker->lock); + } if (!sub->closed) { // If we fail in that call, we will report to async err callback // (if one is specified). s = jsSub_resetOrderedConsumer(sub, sub->jsi->sseq+1); } + if (sub->libDlvWorker != NULL) + { + natsMutex_Unlock(sub->libDlvWorker->lock); + } natsSub_Unlock(sub); } From 6804caef0f26022b059a39ad8877d29aa4ab6227 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Mon, 15 Jul 2024 03:00:27 -0700 Subject: [PATCH 11/12] experiment with double-coverage --- .github/workflows/on-pr-debug.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/on-pr-debug.yml b/.github/workflows/on-pr-debug.yml index 2f1c71e58..089cd7f58 100644 --- a/.github/workflows/on-pr-debug.yml +++ b/.github/workflows/on-pr-debug.yml @@ -28,6 +28,17 @@ jobs: secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + coverage-pooled: + name: "Coverage" + uses: ./.github/workflows/build-test.yml + with: + coverage: ON + server_version: main + type: Debug + lib_msg_delivery: ON + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + dev-mode: name: "DEV_MODE" uses: ./.github/workflows/build-test.yml From bf65830b7411b1da57896e7a0ded34d270452ddd Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Wed, 17 Jul 2024 13:37:47 -0700 Subject: [PATCH 12/12] Fixed test_MicroAsyncErrorHandler_MaxPendingMsgs --- test/test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.c b/test/test.c index 27b441264..12a92c4d5 100644 --- a/test/test.c +++ b/test/test.c @@ -33932,8 +33932,8 @@ test_MicroAsyncErrorHandler_MaxPendingMsgs(void) natsMutex_Lock(arg.m); while ((s != NATS_TIMEOUT) && !arg.closed) s = natsCondition_TimedWait(arg.c, arg.m, 1000); - natsMutex_Unlock(arg.m); testCond((s == NATS_OK) && arg.closed && (arg.status == NATS_SLOW_CONSUMER)); + natsMutex_Unlock(arg.m); microService_Destroy(m); _waitForMicroservicesAllDone(&arg);