Skip to content
Permalink
Browse files
Implement reference counting using C11 atomics and do some minor code…
… cleanup.

The previous implementation using pthread_mutex is flawed for glibc before 2.23 as discussed in https://sourceware.org/bugzilla/show_activity.cgi?id=13690. By using C11 atomics we have a guaranteed correct and possibly more efficient implementation.
  • Loading branch information
PengZheng committed Jan 19, 2022
1 parent 4f3dcfd commit 9753b239130febe16ac6b3a4302d4bca63cf358f
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 186 deletions.
@@ -44,7 +44,7 @@ endif ()
set(ENABLE_MORE_WARNINGS OFF)

# Set C specific flags
set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu11 -fPIC ${CMAKE_C_FLAGS}")
set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")

# Set C++ specific flags
@@ -35,7 +35,9 @@

#include "service_reference_private.h"
#include "service_registration_private.h"
#include "celix_build_assert.h"

static void serviceReference_doRelease(struct celix_ref *);
static void serviceReference_destroy(service_reference_pt);
static void serviceReference_logWarningUsageCountBelowZero(service_reference_pt ref);

@@ -46,15 +48,14 @@ celix_status_t serviceReference_create(registry_callback_t callback, bundle_pt r
if (!ref) {
status = CELIX_ENOMEM;
} else {
celix_ref_init(&ref->refCount);
ref->callback = callback;
ref->referenceOwner = referenceOwner;
ref->registration = registration;
ref->service = NULL;
serviceRegistration_getBundle(registration, &ref->registrationBundle);
celixThreadRwlock_create(&ref->lock, NULL);
ref->refCount = 1;
ref->usageCount = 0;

serviceRegistration_retain(ref->registration);
}

@@ -68,34 +69,28 @@ celix_status_t serviceReference_create(registry_callback_t callback, bundle_pt r
}

celix_status_t serviceReference_retain(service_reference_pt ref) {
celixThreadRwlock_writeLock(&ref->lock);
ref->refCount += 1;
celixThreadRwlock_unlock(&ref->lock);
celix_ref_get(&ref->refCount);
return CELIX_SUCCESS;
}

celix_status_t serviceReference_release(service_reference_pt ref, bool *out) {
bool destroyed = false;
celixThreadRwlock_writeLock(&ref->lock);
assert(ref->refCount > 0);
ref->refCount -= 1;
if (ref->refCount == 0) {
if (ref->registration != NULL) {
serviceRegistration_release(ref->registration);
}
celixThreadRwlock_unlock(&ref->lock);
serviceReference_destroy(ref);
destroyed = true;
} else {
celixThreadRwlock_unlock(&ref->lock);
}

destroyed = celix_ref_put(&ref->refCount, serviceReference_doRelease);
if (out) {
*out = destroyed;
}
return CELIX_SUCCESS;
}

static void serviceReference_doRelease(struct celix_ref *refCount) {
service_reference_pt ref = (service_reference_pt)refCount;
BUILD_ASSERT(offsetof(struct serviceReference, refCount) == 0);
if(ref->registration != NULL) {
serviceRegistration_release(ref->registration);
}
serviceReference_destroy(ref);
}

celix_status_t serviceReference_increaseUsage(service_reference_pt ref, size_t *out) {
//fw_log(logger, CELIX_LOG_LEVEL_DEBUG, "Destroying service reference %p\n", ref);
size_t local = 0;
@@ -144,18 +139,15 @@ celix_status_t serviceReference_getUsageCount(service_reference_pt ref, size_t *
celix_status_t serviceReference_getReferenceCount(service_reference_pt ref, size_t *count) {
celix_status_t status = CELIX_SUCCESS;
celixThreadRwlock_readLock(&ref->lock);
*count = ref->refCount;
*count = ref->refCount.count;
celixThreadRwlock_unlock(&ref->lock);
return status;
}

celix_status_t serviceReference_getService(service_reference_pt ref, void **service) {
celix_status_t serviceReference_getService(service_reference_pt ref, const void **service) {
celix_status_t status = CELIX_SUCCESS;
celixThreadRwlock_readLock(&ref->lock);
/*NOTE the service argument should be 'const void**'
To ensure backwards compatibility a cast is made instead.
*/
*service = (const void**) ref->service;
*service = ref->service;
celixThreadRwlock_unlock(&ref->lock);
return status;
}
@@ -169,7 +161,7 @@ celix_status_t serviceReference_setService(service_reference_pt ref, const void
}

static void serviceReference_destroy(service_reference_pt ref) {
assert(ref->refCount == 0);
assert(ref->refCount.count == 0);
celixThreadRwlock_destroy(&ref->lock);
ref->registration = NULL;
free(ref);
@@ -29,16 +29,16 @@

#include "registry_callback_private.h"
#include "service_reference.h"
#include "celix_ref.h"


struct serviceReference {
struct celix_ref refCount;
registry_callback_t callback;
bundle_pt referenceOwner;
struct serviceRegistration * registration;
bundle_pt registrationBundle;
const void* service;

size_t refCount;
size_t usageCount;

celix_thread_rwlock_t lock;
@@ -59,7 +59,7 @@ celix_status_t serviceReference_getUsageCount(service_reference_pt reference, si
celix_status_t serviceReference_getReferenceCount(service_reference_pt reference, size_t *count);

celix_status_t serviceReference_setService(service_reference_pt ref, const void *service);
celix_status_t serviceReference_getService(service_reference_pt reference, void **service);
celix_status_t serviceReference_getService(service_reference_pt reference, const void **service);

celix_status_t serviceReference_getOwner(service_reference_pt reference, bundle_pt *owner);

@@ -24,11 +24,12 @@

#include "service_registration_private.h"
#include "celix_constants.h"
#include "celix_build_assert.h"

static void serviceRegistration_destroy(struct celix_ref *);
static celix_status_t serviceRegistration_initializeProperties(service_registration_pt registration, properties_pt properties);
static celix_status_t serviceRegistration_createInternal(registry_callback_t callback, bundle_pt bundle, const char* serviceName, unsigned long serviceId,
const void * serviceObject, properties_pt dictionary, enum celix_service_type svcType, service_registration_pt *registration);
static celix_status_t serviceRegistration_destroy(service_registration_pt registration);

service_registration_pt serviceRegistration_create(registry_callback_t callback, bundle_pt bundle, const char* serviceName, unsigned long serviceId, const void * serviceObject, properties_pt dictionary) {
service_registration_pt registration = NULL;
@@ -48,22 +49,13 @@ static celix_status_t serviceRegistration_createInternal(registry_callback_t cal
celix_status_t status = CELIX_SUCCESS;
service_registration_pt reg = calloc(1, sizeof(*reg));
if (reg) {
celix_ref_init(&reg->refCount);
reg->callback = callback;
reg->services = NULL;
reg->nrOfServices = 0;
reg->svcType = svcType;
reg->className = strndup(serviceName, 1024*10);
reg->bundle = bundle;
reg->refCount = 1;
reg->serviceId = serviceId;
reg->svcObj = serviceObject;

if (svcType == CELIX_DEPRECATED_FACTORY_SERVICE) {
reg->deprecatedFactory = (service_factory_pt) reg->svcObj;
} else if (svcType == CELIX_FACTORY_SERVICE) {
reg->factory = (celix_service_factory_t*) reg->svcObj;
}

reg->isUnregistering = false;
celixThreadRwlock_create(&reg->lock, NULL);

@@ -83,35 +75,24 @@ static celix_status_t serviceRegistration_createInternal(registry_callback_t cal
}

void serviceRegistration_retain(service_registration_pt registration) {
celixThreadRwlock_writeLock(&registration->lock);
registration->refCount += 1;
celixThreadRwlock_unlock(&registration->lock);
celix_ref_get(&registration->refCount);
}

void serviceRegistration_release(service_registration_pt registration) {
celixThreadRwlock_writeLock(&registration->lock);
assert(registration->refCount > 0);
registration->refCount -= 1;
if (registration->refCount == 0) {
serviceRegistration_destroy(registration);
} else {
celixThreadRwlock_unlock(&registration->lock);
}
celix_ref_put(&registration->refCount, serviceRegistration_destroy);
}

static celix_status_t serviceRegistration_destroy(service_registration_pt registration) {
//fw_log(logger, CELIX_LOG_LEVEL_DEBUG, "Destroying service registration %p\n", registration);
free(registration->className);
registration->className = NULL;
static void serviceRegistration_destroy(struct celix_ref *refCount) {
service_registration_pt registration = (service_registration_pt )refCount;
BUILD_ASSERT(offsetof(service_registration_t, refCount) == 0);

//fw_log(logger, CELIX_LOG_LEVEL_DEBUG, "Destroying service registration %p\n", registration);
free(registration->className);
registration->className = NULL;
registration->callback.unregister = NULL;

properties_destroy(registration->properties);
celixThreadRwlock_unlock(&registration->lock);
properties_destroy(registration->properties);
celixThreadRwlock_destroy(&registration->lock);
free(registration);

return CELIX_SUCCESS;
free(registration);
}

static celix_status_t serviceRegistration_initializeProperties(service_registration_pt registration, properties_pt dictionary) {
@@ -23,6 +23,7 @@

#include "registry_callback_private.h"
#include "service_registration.h"
#include "celix_ref.h"

enum celix_service_type {
CELIX_PLAIN_SERVICE,
@@ -31,6 +32,7 @@ enum celix_service_type {
};

struct serviceRegistration {
struct celix_ref refCount;
registry_callback_t callback;

char * className;
@@ -41,14 +43,12 @@ struct serviceRegistration {
bool isUnregistering;

enum celix_service_type svcType;
const void * svcObj;
service_factory_pt deprecatedFactory;
celix_service_factory_t *factory;
union {
const void * svcObj;
service_factory_pt deprecatedFactory;
celix_service_factory_t *factory;
};

struct service *services;
int nrOfServices;

size_t refCount; //protected by mutex

celix_thread_rwlock_t lock;
};
@@ -492,7 +492,7 @@ celix_status_t serviceRegistry_ungetServiceReference(service_registry_pt registr
return status;
}

static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry __attribute__((unused)), bundle_pt bundle, service_reference_pt ref, size_t usageCount, size_t refCount) {
static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry, bundle_pt bundle, service_reference_pt ref, size_t usageCount, size_t refCount) {
if (usageCount > 0) {
fw_log(registry->framework->logger, CELIX_LOG_LEVEL_WARNING, "Service Reference destroyed with usage count is %zu, expected 0. Look for missing bundleContext_ungetService calls.", usageCount);
}
@@ -612,10 +612,7 @@ celix_status_t serviceRegistry_getService(service_registry_pt registry, bundle_p

serviceRegistration_release(registration);

/* NOTE the out argument of sr_getService should be 'const void**'
To ensure backwards compatibility a cast is made instead.
*/
serviceReference_getService(reference, (void **)out);
serviceReference_getService(reference, out);

return status;
}
@@ -630,10 +627,7 @@ celix_status_t serviceRegistry_ungetService(service_registry_pt registry, bundle

celix_status_t status = serviceReference_decreaseUsage(reference, &count);
if (count == 0) {
/*NOTE the argument service of sr_getService should be 'const void**'
To ensure backwards compatibility a cast is made instead.
*/
serviceReference_getService(reference, (void**)&service);
serviceReference_getService(reference, &service);
serviceReference_getServiceRegistration(reference, &reg);
if (reg != NULL) {
serviceRegistration_ungetService(reg, bundle, &service);

0 comments on commit 9753b23

Please sign in to comment.