PengZheng commented on a change in pull request #399:
URL: https://github.com/apache/celix/pull/399#discussion_r816548855



##########
File path: libs/framework/src/service_registration.c
##########
@@ -135,71 +131,71 @@ bool serviceRegistration_isValid(service_registration_pt 
registration) {
 
 celix_status_t serviceRegistration_unregister(service_registration_pt 
registration) {
        celix_status_t status = CELIX_SUCCESS;
-
-    bool notValidOrUnregistering;
-    celixThreadRwlock_readLock(&registration->lock);
-    notValidOrUnregistering = !serviceRegistration_isValid(registration) || 
registration->isUnregistering;
-    celixThreadRwlock_unlock(&registration->lock);
-
+    bool unregistering = false;
     registry_callback_t callback;
     callback.unregister = NULL;
-    bundle_pt bundle = NULL;
 
-       if (notValidOrUnregistering) {
-               status = CELIX_ILLEGAL_STATE;
-       } else {
-        celixThreadRwlock_writeLock(&registration->lock);
-        registration->isUnregistering = true;
-        bundle = registration->bundle;
-        callback = registration->callback;
-        celixThreadRwlock_unlock(&registration->lock);
+    // Without any further need of synchronization between callers, 
__ATOMIC_RELAXED should be sufficient to guarantee that only one caller has a 
chance to run.
+    // Strong form of compare-and-swap is used to avoid spurious failure.
+    if(!__atomic_compare_exchange_n(&registration->isUnregistering, 
&unregistering /* expected*/ , true /* desired */,
+                                    false /* weak */, 
__ATOMIC_RELAXED/*success memorder*/, __ATOMIC_RELAXED/*failure memorder*/)) {
+        status = CELIX_ILLEGAL_STATE;
+        goto immediate_return;

Review comment:
       Indeed, it's better to go without goto. Fixed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to