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

enable tsan #427

Merged
merged 17 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 28 additions & 25 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -295,31 +295,34 @@ jobs:
# ulimit -c unlimited -S
# timeout --signal=SIGABRT 150m ./tst/producer_test --gtest_break_on_failure

# thread-sanitizer:
# runs-on: ubuntu-20.04
# permissions:
# id-token: write
# contents: read
# env:
# CC: clang
# CXX: clang++
# AWS_KVS_LOG_LEVEL: 2
# steps:
# - name: Clone repository
# uses: actions/checkout@v3
# - name: Configure AWS Credentials
# uses: aws-actions/configure-aws-credentials@v1-node16
# with:
# role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
# role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
# aws-region: ${{ secrets.AWS_REGION }}
# - name: Build repository
# run: |
# mkdir build && cd build
# cmake .. -DBUILD_TEST=TRUE -DTHREAD_SANITIZER=TRUE -DBUILD_COMMON_LWS=TRUE
# make
# ulimit -c unlimited -S
# timeout --signal=SIGABRT 150m ./tst/producer_test --gtest_break_on_failure
#thread-sanitizer:
# runs-on: ubuntu-20.04
# permissions:
# id-token: write
# contents: read
# env:
# CC: clang
# CXX: clang++
# AWS_KVS_LOG_LEVEL: 2
# steps:
# - name: Clone repository
# uses: actions/checkout@v3
# - name: Configure AWS Credentials
# uses: aws-actions/configure-aws-credentials@v1-node16
# with:
# role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
# role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
# aws-region: ${{ secrets.AWS_REGION }}
# - name: Build repository
# run: |
# mkdir build && cd build
# cmake .. -DBUILD_TEST=TRUE -DTHREAD_SANITIZER=TRUE -DBUILD_COMMON_LWS=TRUE
# make
# - name: Run tests
# run: |
# cd build
# ulimit -c unlimited -S
# timeout --signal=SIGABRT 150m ./tst/producer_test --gtest_break_on_failure

ubuntu-gcc:
runs-on: ubuntu-20.04
Expand Down
2 changes: 1 addition & 1 deletion CMake/Dependencies/libcurl-CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.6.3)

project(libcurl-download NONE)
project(libcurl-download LANGUAGES C)

find_program(MAKE_EXE NAMES make)

Expand Down
1 change: 0 additions & 1 deletion CMake/Dependencies/libgtest-CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ cmake_minimum_required(VERSION 3.6.3)
project(libgtest-download NONE)

include(ExternalProject)

ExternalProject_Add(libgtest-download
GIT_REPOSITORY https://github.com/google/googletest.git
GIT_TAG release-1.12.1
Expand Down
2 changes: 1 addition & 1 deletion CMake/Dependencies/libkvspic-CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.6.3)

project(libkvspic-download NONE)
project(libkvspic-download LANGUAGES C)

include(ExternalProject)

Expand Down
2 changes: 1 addition & 1 deletion CMake/Dependencies/libmbedtls-CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.6.3)

project(libmbedtls-download NONE)
project(libmbedtls-download LANGUAGES C)

include(ExternalProject)

Expand Down
2 changes: 1 addition & 1 deletion CMake/Dependencies/libwebsockets-CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.6.3)

project(libwebsocket-download NONE)
project(libwebsocket-download LANGUAGES C)

include(ExternalProject)

Expand Down
2 changes: 1 addition & 1 deletion src/source/Common/AwsV4Signer.c
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ STATUS generateSignatureDateTime(UINT64 currentTime, PCHAR pDateTimeStr)

// Convert to time_t
timeT = (time_t) (currentTime / HUNDREDS_OF_NANOS_IN_A_SECOND);
retSize = STRFTIME(pDateTimeStr, SIGNATURE_DATE_TIME_STRING_LEN, DATE_TIME_STRING_FORMAT, GMTIME(&timeT));
retSize = STRFTIME(pDateTimeStr, SIGNATURE_DATE_TIME_STRING_LEN, DATE_TIME_STRING_FORMAT, GMTIME_THREAD_SAFE(&timeT));
CHK(retSize > 0, STATUS_BUFFER_TOO_SMALL);
pDateTimeStr[retSize] = '\0';

Expand Down
33 changes: 21 additions & 12 deletions src/source/CurlApiCallbacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -1005,15 +1005,17 @@ STATUS createStreamCurl(UINT64 customData, PCHAR deviceName, PCHAR streamName, P

CleanUp:

if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (STATUS_FAILED(retStatus)) {
if (IS_VALID_TID_VALUE(threadId)) {
THREAD_CANCEL(threadId);
}

freeCurlRequest(&pCurlRequest);
} else if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (shutdownLocked) {
Expand Down Expand Up @@ -1237,15 +1239,17 @@ STATUS describeStreamCurl(UINT64 customData, PCHAR streamName, PServiceCallConte

CleanUp:

if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (STATUS_FAILED(retStatus)) {
if (IS_VALID_TID_VALUE(threadId)) {
THREAD_CANCEL(threadId);
}

freeCurlRequest(&pCurlRequest);
} else if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (shutdownLocked) {
Expand Down Expand Up @@ -1548,15 +1552,18 @@ STATUS getStreamingEndpointCurl(UINT64 customData, PCHAR streamName, PCHAR apiNa

CleanUp:

if (startLocked) {
// Release the lock to let the awaiting handler thread to continue.
// This needs to be done before freeCurlRequest because there we will free the startLock mutex
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (STATUS_FAILED(retStatus)) {
if (IS_VALID_TID_VALUE(threadId)) {
THREAD_CANCEL(threadId);
}

freeCurlRequest(&pCurlRequest);
} else if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (shutdownLocked) {
Expand Down Expand Up @@ -1877,15 +1884,17 @@ STATUS tagResourceCurl(UINT64 customData, PCHAR streamArn, UINT32 tagCount, PTag

CleanUp:

if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (STATUS_FAILED(retStatus)) {
if (IS_VALID_TID_VALUE(threadId)) {
THREAD_CANCEL(threadId);
}

freeCurlRequest(&pCurlRequest);
} else if (startLocked) {
// Release the lock to let the awaiting handler thread to continue
pCallbacksProvider->clientCallbacks.unlockMutexFn(pCallbacksProvider->clientCallbacks.customData, pCurlRequest->startLock);
}

if (shutdownLocked) {
Expand Down
10 changes: 5 additions & 5 deletions src/source/Response.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ STATUS createCurlResponse(PCurlRequest pCurlRequest, PCurlResponse* ppCurlRespon

// init putMedia related members
pCurlResponse->endOfStream = FALSE;
pCurlResponse->paused = TRUE;
ATOMIC_STORE_BOOL(&pCurlResponse->paused, TRUE);
pCurlResponse->debugDumpFile = FALSE;
pCurlResponse->debugDumpFilePath[0] = '\0';

Expand Down Expand Up @@ -456,8 +456,8 @@ STATUS notifyDataAvailable(PCurlResponse pCurlResponse, UINT64 durationAvailable
DLOGV("[%s] Note data received: duration(100ns): %" PRIu64 " bytes %" PRIu64 " for stream handle %" PRIu64,
pCurlResponse->pCurlRequest->streamName, durationAvailable, sizeAvailable, pCurlResponse->pCurlRequest->uploadHandle);

if (pCurlResponse->paused && pCurlResponse->pCurl != NULL) {
pCurlResponse->paused = FALSE;
if (ATOMIC_LOAD_BOOL(&pCurlResponse->paused) && pCurlResponse->pCurl != NULL) {
ATOMIC_STORE_BOOL(&pCurlResponse->paused, FALSE);
// frequent pause unpause causes curl segfault in offline scenario
THREAD_SLEEP(10 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND);
// un-pause curl
Expand Down Expand Up @@ -634,7 +634,7 @@ SIZE_T postReadCallback(PCHAR pBuffer, SIZE_T size, SIZE_T numItems, PVOID custo
pCurlApiCallbacks = pCurlRequest->pCurlApiCallbacks;
uploadHandle = pCurlResponse->pCurlRequest->uploadHandle;

if (pCurlResponse->paused) {
if (ATOMIC_LOAD_BOOL(&pCurlResponse->paused)) {
bytesWritten = CURL_READFUNC_PAUSE;
CHK(FALSE, retStatus);
}
Expand Down Expand Up @@ -721,7 +721,7 @@ SIZE_T postReadCallback(PCHAR pBuffer, SIZE_T size, SIZE_T numItems, PVOID custo
}
}
} else if (bytesWritten == CURL_READFUNC_PAUSE) {
pCurlResponse->paused = TRUE;
ATOMIC_STORE_BOOL(&pCurlResponse->paused, TRUE);
}

// Since curl is about to terminate gracefully, set flag to prevent shutdown thread from timing it out.
Expand Down
2 changes: 1 addition & 1 deletion src/source/Response.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ struct __CurlResponse {
BOOL endOfStream;

// Whether curl is paused
volatile BOOL paused;
volatile ATOMIC_BOOL paused;

// Whether to dump streaming session into mkv file
BOOL debugDumpFile;
Expand Down
Loading
Loading