This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/celix.git
The following commit(s) were added to refs/heads/develop by this push:
new 03c7f31 Fix data race in get/set bundle state (#214)
03c7f31 is described below
commit 03c7f31c2462ff33b234d456be2625ba6527ad05
Author: Michael de Lang <[email protected]>
AuthorDate: Tue May 5 11:48:29 2020 +0000
Fix data race in get/set bundle state (#214)
* Fix data race in get/set bundle state
* Add option to enable TSAN and fix celix_threads test data race
---
cmake/celix_project/CelixProject.cmake | 6 ++++++
libs/framework/src/bundle.c | 8 ++++----
libs/utils/private/test/celix_threads_test.cpp | 28 +++++++++++++-------------
libs/utils/src/celix_threads.c | 4 ++--
4 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/cmake/celix_project/CelixProject.cmake
b/cmake/celix_project/CelixProject.cmake
index 30ae2b8..7c53902 100644
--- a/cmake/celix_project/CelixProject.cmake
+++ b/cmake/celix_project/CelixProject.cmake
@@ -17,6 +17,7 @@
option(ENABLE_ADDRESS_SANITIZER "Enabled building with address sanitizer. Note
for gcc libasan must be installed," OFF)
option(ENABLE_UNDEFINED_SANITIZER "Enabled building with undefined behavior
sanitizer." OFF)
+option(ENABLE_THREAD_SANITIZER "Enabled building with thread sanitizer." OFF)
if (ENABLE_ADDRESS_SANITIZER)
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR
"${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
@@ -33,6 +34,11 @@ if (ENABLE_UNDEFINED_SANITIZER)
set(CMAKE_CXX_FLAGS "-fsanitize=undefined ${CMAKE_CXX_FLAGS}")
endif()
+if (ENABLE_THREAD_SANITIZER)
+ set(CMAKE_C_FLAGS "-fsanitize=thread ${CMAKE_C_FLAGS}")
+ set(CMAKE_CXX_FLAGS "-fsanitize=thread ${CMAKE_CXX_FLAGS}")
+endif()
+
MACRO(celix_subproject)
set(ARGS "${ARGN}")
diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c
index f59e86a..e5e2032 100644
--- a/libs/framework/src/bundle.c
+++ b/libs/framework/src/bundle.c
@@ -177,12 +177,12 @@ celix_status_t bundle_getState(const_bundle_pt bundle,
bundle_state_e *state) {
*state = OSGI_FRAMEWORK_BUNDLE_UNKNOWN;
return CELIX_BUNDLE_EXCEPTION;
}
- *state = bundle->state;
+ __atomic_load(&bundle->state, state, __ATOMIC_ACQUIRE);
return CELIX_SUCCESS;
}
celix_status_t bundle_setState(bundle_pt bundle, bundle_state_e state) {
- bundle->state = state;
+ __atomic_store_n(&bundle->state, state, __ATOMIC_RELEASE);
return CELIX_SUCCESS;
}
@@ -495,7 +495,7 @@ celix_status_t bundle_refresh(bundle_pt bundle) {
if (status == CELIX_SUCCESS) {
status = bundle_addModule(bundle, module);
if (status == CELIX_SUCCESS) {
- bundle->state = OSGI_FRAMEWORK_BUNDLE_INSTALLED;
+ __atomic_store_n(&bundle->state,
OSGI_FRAMEWORK_BUNDLE_INSTALLED, __ATOMIC_RELEASE);
}
}
}
@@ -612,7 +612,7 @@ long celix_bundle_getId(const bundle_t* bnd) {
}
celix_bundle_state_e celix_bundle_getState(const bundle_t *bnd) {
- return bnd->state;
+ return __atomic_load_n(&bnd->state, __ATOMIC_ACQUIRE);
}
char* celix_bundle_getEntry(const bundle_t* bnd, const char *path) {
diff --git a/libs/utils/private/test/celix_threads_test.cpp
b/libs/utils/private/test/celix_threads_test.cpp
index 0c404c0..e5f022b 100644
--- a/libs/utils/private/test/celix_threads_test.cpp
+++ b/libs/utils/private/test/celix_threads_test.cpp
@@ -61,8 +61,8 @@ static int celix_thread_t_equals(const void * object, const
void * compareTo){
celix_thread_t * thread1 = (celix_thread_t*) object;
celix_thread_t * thread2 = (celix_thread_t*) compareTo;
- return thread1->thread == thread2->thread &&
- thread1->threadInitialized == thread2->threadInitialized;
+ return __atomic_load_n(&thread1->thread, __ATOMIC_ACQUIRE) ==
__atomic_load_n(&thread2->thread, __ATOMIC_ACQUIRE) &&
+ __atomic_load_n(&thread1->threadInitialized, __ATOMIC_ACQUIRE) ==
__atomic_load_n(&thread2->threadInitialized, __ATOMIC_ACQUIRE);
}
static const char * celix_thread_t_toString(const void * object){
@@ -79,7 +79,7 @@ static void * thread_test_func_exit(void *);
static void * thread_test_func_detach(void *);
static void * thread_test_func_self(void *);
static void * thread_test_func_once(void*);
-static void thread_test_func_once_init(void);
+static void thread_test_func_once_init();
static void * thread_test_func_lock(void *);
static void * thread_test_func_cond_wait(void *);
static void * thread_test_func_cond_broadcast(void *);
@@ -105,10 +105,10 @@ int main(int argc, char** argv) {
TEST_GROUP(celix_thread) {
celix_thread thread;
- void setup(void) {
+ void setup() override {
}
- void teardown(void) {
+ void teardown() override {
}
};
@@ -116,7 +116,7 @@ TEST_GROUP(celix_thread_kill) {
celix_thread thread;
struct sigaction sigact, sigactold;
- void setup(void) {
+ void setup() override {
memset(&sigact, 0, sizeof(sigact));
sigact.sa_handler = thread_test_func_kill_handler;
sigaction(SIGUSR1, &sigact, &sigactold);
@@ -124,7 +124,7 @@ TEST_GROUP(celix_thread_kill) {
mock_c()->installComparator("celix_thread_t", celix_thread_t_equals,
celix_thread_t_toString);
}
- void teardown(void) {
+ void teardown() override {
sigaction(SIGUSR1, &sigactold, &sigact);
mock_c()->removeAllComparatorsAndCopiers();
@@ -135,10 +135,10 @@ TEST_GROUP(celix_thread_mutex) {
celix_thread thread;
celix_thread_mutex_t mu;
- void setup(void) {
+ void setup() override {
}
- void teardown(void) {
+ void teardown() override {
}
};
@@ -147,20 +147,20 @@ TEST_GROUP(celix_thread_condition) {
celix_thread_mutex_t mu;
celix_thread_cond_t cond;
- void setup(void) {
+ void setup() {
}
- void teardown(void) {
+ void teardown() {
}
};
TEST_GROUP(celix_thread_rwlock) {
celix_thread thread;
- void setup(void) {
+ void setup() {
}
- void teardown(void) {
+ void teardown() {
}
};
@@ -464,7 +464,7 @@ static void * thread_test_func_once(void * arg) {
return NULL;
}
-static void thread_test_func_once_init(void) {
+static void thread_test_func_once_init() {
mock_c()->actualCall("thread_test_func_once_init");
}
diff --git a/libs/utils/src/celix_threads.c b/libs/utils/src/celix_threads.c
index 03b777c..792170d 100644
--- a/libs/utils/src/celix_threads.c
+++ b/libs/utils/src/celix_threads.c
@@ -38,7 +38,7 @@ celix_status_t celixThread_create(celix_thread_t *new_thread,
celix_thread_attr_
status = CELIX_BUNDLE_EXCEPTION;
}
else {
- (*new_thread).threadInitialized = true;
+ __atomic_store_n(&(*new_thread).threadInitialized, true,
__ATOMIC_RELEASE);
}
return status;
@@ -94,7 +94,7 @@ int celixThread_equals(celix_thread_t thread1, celix_thread_t
thread2) {
}
bool celixThread_initialized(celix_thread_t thread) {
- return thread.threadInitialized;
+ return __atomic_load_n(&thread.threadInitialized, __ATOMIC_ACQUIRE);
}
