gh-87: Support lock stealing from a lock guard.

Fix race condition in `celix_bundleContext_getDependencyManager` and avoid write lock when possible.
apache · Feb 27, 2024 · b2d197e · b2d197e
1 parent 13fb823
commit b2d197e
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c
@@ -517,19 +517,23 @@ void celix_bundleContext_waitForAsyncUnregistration(celix_bundle_context_t* ctx,
 }
 
 celix_dependency_manager_t* celix_bundleContext_getDependencyManager(bundle_context_t *ctx) {
-    celix_dependency_manager_t* result = NULL;
-    if (ctx != NULL) {
-        celixThreadRwlock_readLock(&ctx->lock);
-        if (ctx->mng == NULL) {
-            ctx->mng = celix_private_dependencyManager_create(ctx);
-        }
-        if (ctx->mng == NULL) {
-            framework_logIfError(ctx->framework->logger, CELIX_BUNDLE_EXCEPTION, NULL, "Cannot create dependency manager");
-        }
-        result = ctx->mng;
-        celixThreadRwlock_unlock(&ctx->lock);
+    if (ctx == NULL) {
+        return NULL;
     }
-    return result;
+    celix_auto(celix_rwlock_rlock_guard_t) rlockGuard = celixRwlockRlockGuard_init(&ctx->lock);
+    if (ctx->mng) {
+        return ctx->mng;
+    }
+    celixThreadRwlock_unlock(celix_steal_ptr(rlockGuard.lock));
+
+    celix_auto(celix_rwlock_wlock_guard_t) wlockGuard = celixRwlockWlockGuard_init(&ctx->lock);
+    if (ctx->mng == NULL) {
+        ctx->mng = celix_private_dependencyManager_create(ctx);
+    }
+    if (ctx->mng == NULL) {
+        framework_logIfError(ctx->framework->logger, CELIX_BUNDLE_EXCEPTION, NULL, "Cannot create dependency manager");
+    }
+    return ctx->mng;
 }
 
 static celix_status_t bundleContext_bundleChanged(void* listenerSvc, bundle_event_t* event) {

diff --git a/libs/utils/include/celix_threads.h b/libs/utils/include/celix_threads.h
@@ -139,7 +139,9 @@ static CELIX_UNUSED inline celix_mutex_lock_guard_t celixMutexLockGuard_init(cel
  * @param guard A celix_mutex_lock_guard_t.
  */
 static CELIX_UNUSED inline void celixMutexLockGuard_deinit(celix_mutex_lock_guard_t* guard) {
-    celixThreadMutex_unlock(guard->mutex);
+    if (guard->mutex) {
+        celixThreadMutex_unlock(guard->mutex);
+    }
 }
 
 CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_mutex_lock_guard_t, celixMutexLockGuard_deinit)
@@ -211,7 +213,9 @@ static CELIX_UNUSED inline celix_rwlock_wlock_guard_t celixRwlockWlockGuard_init
  * @param guard A celix_rwlock_wlock_guard_t.
  */
 static CELIX_UNUSED inline void celixRwlockWlockGuard_deinit(celix_rwlock_wlock_guard_t* guard) {
-    celixThreadRwlock_unlock(guard->lock);
+    if (guard->lock) {
+        celixThreadRwlock_unlock(guard->lock);
+    }
 }
 
 CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_rwlock_wlock_guard_t, celixRwlockWlockGuard_deinit)
@@ -256,7 +260,9 @@ static CELIX_UNUSED inline celix_rwlock_rlock_guard_t celixRwlockRlockGuard_init
  * @param guard A celix_rwlock_rlock_guard_t.
  */
 static CELIX_UNUSED inline void celixRwlockRlockGuard_deinit(celix_rwlock_rlock_guard_t* guard) {
-    celixThreadRwlock_unlock(guard->lock);
+    if (guard->lock) {
+        celixThreadRwlock_unlock(guard->lock);
+    }
 }
 
 CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_rwlock_rlock_guard_t, celixRwlockRlockGuard_deinit)