Skip to content

Commit

Permalink
Fix TSAN issues in thread support library
Browse files Browse the repository at this point in the history
--------------------------------------------------------

The issue was the test is attempting to check for change in variable through a while loop, but did not protect the data read using mutext lock. This could potentially lead to reading of unexpected data and cause a tsan error.

The fix wraps the checks for variable state inside the mutex lock.
  • Loading branch information
sherry-yuan committed Mar 4, 2022
1 parent 1dd236b commit 329a433
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
1 change: 1 addition & 0 deletions lib/acl_threadsupport/test/acl_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ SimpleString StringFrom(uint32_t x);
SimpleString StringFrom(uint64_t x);
SimpleString StringFrom(intptr_t x);
SimpleString StringFrom(size_t x);
bool check_threadtest_state(int state, bool check_eq);
18 changes: 15 additions & 3 deletions lib/acl_threadsupport/test/acl_threadsupport_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ volatile int threadvar = 0;

void *test_thread(void *) {
while (1) {
while (threadtest_state == SERVER_STATE)
while (check_threadtest_state(SERVER_STATE, true))
acl_thread_yield();
if (threadtest_state == END_STATE) {
return 0;
Expand All @@ -108,6 +108,18 @@ void *test_thread(void *) {
}
}

bool check_threadtest_state(int state, bool check_eq) {
bool result;
acl_mutex_lock(&mymutex);
if (check_eq) {
result = threadtest_state == state;
} else {
result = threadtest_state != state;
}
acl_mutex_unlock(&mymutex);
return result;
}

TEST(threadsupport, threads) {
// Setup
int res = 0;
Expand All @@ -125,15 +137,15 @@ TEST(threadsupport, threads) {
res = acl_mutex_unlock(&mymutex);
CHECK_EQUAL(0, res);

while (threadtest_state != SERVER_STATE)
while (check_threadtest_state(SERVER_STATE, false))
acl_thread_yield();
res = acl_mutex_lock(&mymutex);
CHECK_EQUAL(0, res);
CHECK_EQUAL(threadvar, 7 + 13);
threadtest_state = CLIENT_STATE;
res = acl_mutex_unlock(&mymutex);
CHECK_EQUAL(0, res);
while (threadtest_state != SERVER_STATE)
while (check_threadtest_state(SERVER_STATE, false))
acl_thread_yield();
res = acl_mutex_lock(&mymutex);
CHECK_EQUAL(0, res);
Expand Down

0 comments on commit 329a433

Please sign in to comment.