Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIXED] cleaning up sanitize=thread found several races #771

Merged
merged 14 commits into from
Jul 22, 2024
Merged
20 changes: 19 additions & 1 deletion .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ on:
coverage:
type: string
default: "OFF"
dev_mode:
type: string
default: "OFF"
lib_msg_delivery:
type: string
default: "OFF"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
levb marked this conversation as resolved.
Show resolved Hide resolved
if [[ $(grep -q 'ThreadSanitizer: ' /tmp/out.txt; echo $?) == 0 ]]; then
echo "!!! ThreadSanitizer detected WARNING(s) !!!"
exit 1
Expand Down
13 changes: 13 additions & 0 deletions .github/workflows/on-pr-debug.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -56,6 +68,7 @@ jobs:
name: "Sanitize thread"
uses: ./.github/workflows/build-test.yml
with:
type: Debug
sanitize: thread
server_version: main

Expand Down
23 changes: 23 additions & 0 deletions .github/workflows/on-push-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Not sure why you would shorten to san- here while you used santize- for the address matrix run above :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shorter is better? :) (will change for consistency)

name: "Sanitize thread"
uses: ./.github/workflows/build-test.yml
with:
sanitize: thread
server_version: main
21 changes: 20 additions & 1 deletion test/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -5477,6 +5477,7 @@ test_natsSrvVersionAtLeast(void)
{
s = NATS_ERR;
}
natsConn_Unlock(nc);
}
}
testCond(s == NATS_OK);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
levb marked this conversation as resolved.
Show resolved Hide resolved

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");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -29485,6 +29500,9 @@ _jsOrderedErrHandler(natsConnection *nc, natsSubscription *subscription, natsSta
{
struct threadArg *args = (struct threadArg*) closure;

if (err != NATS_MISSED_HEARTBEAT)
levb marked this conversation as resolved.
Show resolved Hide resolved
return;

natsMutex_Lock(args->m);
args->status = err;
natsCondition_Signal(args->c);
Expand Down Expand Up @@ -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: ");
Expand Down