You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by pn...@apache.org on 2020/09/08 18:50:11 UTC
[celix] branch hotfix/rsa_deadlock created (now 5bf1426)
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a change to branch hotfix/rsa_deadlock
in repository https://gitbox.apache.org/repos/asf/celix.git.
at 5bf1426 Adds a stop service export thread for rsa to prevent deadlocks
This branch includes the following new commits:
new 333b874 Adds test for dynamically creating and destroying a component providing a remote service which deadlocks
new 5bf1426 Adds a stop service export thread for rsa to prevent deadlocks
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[celix] 01/02: Adds test for dynamically creating and destroying a
component providing a remote service which deadlocks
Posted by pn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch hotfix/rsa_deadlock
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 333b8749cae748530dde7b33d17482e115645f8b
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Tue Sep 8 18:26:21 2020 +0200
Adds test for dynamically creating and destroying a component providing a remote service which deadlocks
---
.../gtest/src/rsa_client_server_tests.cc | 12 +++++++++++
.../gtest/src/tst_activator.c | 24 ++++++++++++++++++++++
.../gtest/src/tst_service.h | 1 +
3 files changed, 37 insertions(+)
diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc b/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc
index 9c42951..b8ef8f0 100644
--- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc
+++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc
@@ -136,6 +136,13 @@ extern "C" {
ASSERT_TRUE(ok);
};
+ static void testCreateDestroyComponentWithRemoteService(void *handle __attribute__((unused)), void *svc) {
+ auto *tst = static_cast<tst_service_t *>(svc);
+
+ bool ok = tst->testCreateDestroyComponentWithRemoteService(tst->handle);
+ ASSERT_TRUE(ok);
+ };
+
}
template<typename F>
@@ -188,3 +195,8 @@ TEST_F(RsaDfiClientServerTests, TestRemoteEnum) {
TEST_F(RsaDfiClientServerTests, TestRemoteAction) {
test(testAction);
}
+
+TEST_F(RsaDfiClientServerTests, CreateDestroyComponentWithRemoteService) {
+ test(testCreateDestroyComponentWithRemoteService);
+}
+
diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
index 6385dde..3724755 100644
--- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
+++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
@@ -40,6 +40,7 @@
} while(0)
struct activator {
+ celix_bundle_context_t *ctx;
long svcId;
struct tst_service testSvc;
@@ -269,8 +270,30 @@ static bool bndTestRemoteComplex(void *handle) {
return ok;
}
+static bool bndTestCreateDestroyComponentWithRemoteService(void *handle) {
+ struct activator *act = handle;
+
+ celix_properties_t *properties = celix_properties_create();
+ celix_properties_set(properties, "service.exported.interfaces", CALCULATOR_SERVICE);
+
+ calculator_service_t calcSvc;
+ calcSvc.handle = NULL;
+ calcSvc.add = NULL; //note for this test case the actual services can be NULL
+ calcSvc.sub = NULL; //note for this test case the actual services can be NULL
+ calcSvc.sqrt = NULL; //note for this test case the actual services can be NULL
+
+ celix_dm_component_t *cmp = celix_dmComponent_create(act->ctx, "test");
+ celix_dmComponent_addInterface(cmp, CALCULATOR_SERVICE, NULL, &calcSvc, properties);
+
+ celix_dependency_manager_t *dm = celix_bundleContext_getDependencyManager(act->ctx);
+ dependencyManager_add(dm, cmp);
+ dependencyManager_removeAllComponents(dm); //note should not deadlock
+ return true;
+}
+
static celix_status_t bndStart(struct activator *act, celix_bundle_context_t* ctx) {
//initialize service struct
+ act->ctx = ctx;
act->testSvc.handle = act;
act->testSvc.isCalcDiscovered = bndIsCalculatorDiscovered;
act->testSvc.isRemoteExampleDiscovered = bndIsRemoteExampleDiscovered;
@@ -281,6 +304,7 @@ static celix_status_t bndStart(struct activator *act, celix_bundle_context_t* ct
act->testSvc.testRemoteEnum = bndTestRemoteEnum;
act->testSvc.testRemoteAction = bndTestRemoteAction;
act->testSvc.testRemoteComplex = bndTestRemoteComplex;
+ act->testSvc.testCreateDestroyComponentWithRemoteService = bndTestCreateDestroyComponentWithRemoteService;
//create mutex
diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_service.h b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_service.h
index 596ab63..9dab7c1 100644
--- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_service.h
+++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_service.h
@@ -33,6 +33,7 @@ struct tst_service {
bool (*testRemoteEnum)(void *handle);
bool (*testRemoteAction)(void *handle);
bool (*testRemoteComplex)(void *handle);
+ bool (*testCreateDestroyComponentWithRemoteService)(void *handle);
};
typedef struct tst_service tst_service_t;
[celix] 02/02: Adds a stop service export thread for rsa to prevent
deadlocks
Posted by pn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch hotfix/rsa_deadlock
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 5bf1426256ea53a6cc68c9a861c37cb62682d08c
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Tue Sep 8 20:49:12 2020 +0200
Adds a stop service export thread for rsa to prevent deadlocks
---
.../src/export_registration_dfi.c | 24 +++---
.../src/export_registration_dfi.h | 2 +-
.../src/remote_service_admin_dfi.c | 94 +++++++++++++++++++++-
3 files changed, 104 insertions(+), 16 deletions(-)
diff --git a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c
index 39b43b3..5afa1a3 100644
--- a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c
+++ b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c
@@ -19,7 +19,6 @@
#include <jansson.h>
#include <dyn_interface.h>
-#include <json_serializer.h>
#include <remote_constants.h>
#include <remote_service_admin.h>
#include <service_tracker_customizer.h>
@@ -44,6 +43,7 @@ struct export_registration {
celix_thread_mutex_t mutex;
+ bool active; //protected by mutex
void *service; //protected by mutex
long trackerId; //protected by mutex
@@ -86,6 +86,7 @@ celix_status_t exportRegistration_create(celix_log_helper_t *helper, service_ref
reg->logFile = logFile;
reg->servId = strndup(servId, 1024);
reg->trackerId = -1L;
+ reg->active = true;
remoteInterceptorsHandler_create(context, ®->interceptorsHandler);
@@ -139,8 +140,11 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data
bool cont = remoteInterceptorHandler_invokePreExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, &metadata);
if (cont) {
celixThreadMutex_lock(&export->mutex);
- if (export->service != NULL) {
+ if (export->active && export->service != NULL) {
status = jsonRpc_call(export->intf, export->service, data, responseOut);
+ } else if (!export->active) {
+ status = CELIX_ILLEGAL_STATE;
+ celix_logHelper_warning(export->helper, "Cannot call an inactive service export");
} else {
status = CELIX_ILLEGAL_STATE;
celix_logHelper_error(export->helper, "export service pointer is NULL");
@@ -245,7 +249,7 @@ celix_status_t exportRegistration_start(export_registration_t *reg) {
celixThreadMutex_unlock(®->mutex);
if (prevTrkId >= 0) {
- celix_logHelper_error(reg->helper, "Error staring export registration. The export registration already has an active service tracker");
+ celix_logHelper_error(reg->helper, "Error starting export registration. The export registration already had an active service tracker");
celix_bundleContext_stopTracker(reg->context, prevTrkId);
}
@@ -268,6 +272,12 @@ celix_status_t exportRegistration_stop(export_registration_t *reg) {
return status;
}
+void exportRegistration_setActive(export_registration_t *reg, bool active) {
+ celixThreadMutex_lock(®->mutex);
+ reg->active = active;
+ celixThreadMutex_unlock(®->mutex);
+}
+
static void exportRegistration_addServ(void *data, void *service) {
export_registration_t *reg = data;
celixThreadMutex_lock(®->mutex);
@@ -284,14 +294,6 @@ static void exportRegistration_removeServ(void *data, void *service) {
celixThreadMutex_unlock(®->mutex);
}
-
-celix_status_t exportRegistration_close(export_registration_t *reg) {
- celix_status_t status = CELIX_SUCCESS;
- exportRegistration_stop(reg);
- return status;
-}
-
-
celix_status_t exportRegistration_getException(export_registration_t *registration) {
celix_status_t status = CELIX_SUCCESS;
//TODO
diff --git a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h
index 2333c4c..8bc40eb 100644
--- a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h
+++ b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h
@@ -26,11 +26,11 @@
#include "endpoint_description.h"
celix_status_t exportRegistration_create(celix_log_helper_t *helper, service_reference_pt reference, endpoint_description_t *endpoint, celix_bundle_context_t *context, FILE *logFile, export_registration_t **registration);
-celix_status_t exportRegistration_close(export_registration_t *registration);
void exportRegistration_destroy(export_registration_t *registration);
celix_status_t exportRegistration_start(export_registration_t *registration);
celix_status_t exportRegistration_stop(export_registration_t *registration);
+void exportRegistration_setActive(export_registration_t *registration, bool active);
celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t *metadata, char **response, int *responseLength);
diff --git a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c
index f690a31..53620df 100644
--- a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c
+++ b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c
@@ -19,6 +19,7 @@
#include <stdio.h>
#include <stdlib.h>
+#include <unistd.h>
#include <arpa/inet.h>
#include <netdb.h>
@@ -56,6 +57,16 @@
#define RSA_LOG_DEBUG(admin, msg, ...) \
celix_logHelper_log((admin)->loghelper, CELIX_LOG_LEVEL_ERROR, (msg), ##__VA_ARGS__)
+
+/**
+ * If set to true the rsa will create a thread to handle stopping of service export.
+ *
+ * This stop thread can be removed when the branch feature/async_svc_registration is merged and
+ * celix_bundleContext_stopTrackerAsync is available.
+ *
+ */
+#define CELIX_RSA_USE_STOP_EXPORT_THREAD false
+
struct remote_service_admin {
celix_bundle_context_t *context;
celix_log_helper_t *loghelper;
@@ -63,6 +74,13 @@ struct remote_service_admin {
celix_thread_rwlock_t exportedServicesLock;
hash_map_pt exportedServices;
+ //NOTE stopExportsMutex, stopExports, stopExportsActive, stopExportsCond and stopExportsThread are only used if CELIX_RSA_USE_STOP_EXPORT_THREAD is set to true
+ celix_thread_mutex_t stopExportsMutex;
+ celix_array_list_t *stopExports;
+ bool stopExportsActive;
+ celix_thread_cond_t stopExportsCond;
+ celix_thread_t stopExportsThread;
+
celix_thread_mutex_t importedServicesLock;
array_list_pt importedServices;
@@ -110,6 +128,8 @@ static celix_status_t remoteServiceAdmin_getIpAddress(char* interface, char** ip
static size_t remoteServiceAdmin_readCallback(void *ptr, size_t size, size_t nmemb, void *userp);
static size_t remoteServiceAdmin_write(void *contents, size_t size, size_t nmemb, void *userp);
static void remoteServiceAdmin_log(remote_service_admin_t *admin, int level, const char *file, int line, const char *msg, ...);
+static void remoteServiceAdmin_setupStopExportsThread(remote_service_admin_t* admin);
+static void remoteServiceAdmin_teardownStopExportsThread(remote_service_admin_t* admin);
static void remoteServiceAdmin_curlshare_lock(CURL *handle, curl_lock_data data, curl_lock_access laccess, void *userptr)
{
@@ -220,6 +240,8 @@ celix_status_t remoteServiceAdmin_create(celix_bundle_context_t *context, remote
status = EPERM;
}
+ remoteServiceAdmin_setupStopExportsThread(*admin);
+
// Prepare callbacks structure. We have only one callback, the rest are NULL.
struct mg_callbacks callbacks;
memset(&callbacks, 0, sizeof(callbacks));
@@ -285,6 +307,52 @@ celix_status_t remoteServiceAdmin_destroy(remote_service_admin_t **admin)
return status;
}
+void* remoteServiceAdmin_stopExportsThread(void *data) {
+ remote_service_admin_t* admin = data;
+ bool active = true;
+
+ while (active) {
+ celixThreadMutex_lock(&admin->stopExportsMutex);
+ if (admin->stopExportsActive && celix_arrayList_size(admin->stopExports) == 0) {
+ celixThreadCondition_timedwaitRelative(&admin->stopExportsCond, &admin->stopExportsMutex, 1, 0);
+ }
+ for (int i = 0; i < celix_arrayList_size(admin->stopExports); ++i) {
+ export_registration_t *export = celix_arrayList_get(admin->stopExports, i);
+ exportRegistration_stop(export);
+ exportRegistration_destroy(export);
+ }
+ celix_arrayList_clear(admin->stopExports);
+ active = admin->stopExportsActive;
+ celixThreadMutex_unlock(&admin->stopExportsMutex);
+ }
+
+ return NULL;
+}
+
+static void remoteServiceAdmin_setupStopExportsThread(remote_service_admin_t* admin) {
+ if (CELIX_RSA_USE_STOP_EXPORT_THREAD) {
+ //setup exports stop thread
+ celixThreadMutex_create(&admin->stopExportsMutex, NULL);
+ admin->stopExports = celix_arrayList_create();
+ celixThreadCondition_init(&admin->stopExportsCond, NULL);
+ admin->stopExportsActive = true;
+ celixThread_create(&admin->stopExportsThread, NULL, remoteServiceAdmin_stopExportsThread, admin);
+ }
+}
+
+static void remoteServiceAdmin_teardownStopExportsThread(remote_service_admin_t* admin) {
+ if (CELIX_RSA_USE_STOP_EXPORT_THREAD) {
+ celixThreadMutex_lock(&admin->stopExportsMutex);
+ admin->stopExportsActive = false;
+ celixThreadCondition_broadcast(&admin->stopExportsCond);
+ celixThreadMutex_unlock(&admin->stopExportsMutex);
+ celixThread_join(admin->stopExportsThread, NULL);
+ celix_arrayList_destroy(admin->stopExports);
+ celixThreadMutex_destroy(&admin->stopExportsMutex);
+ celixThreadCondition_destroy(&admin->stopExportsCond);
+ }
+}
+
celix_status_t remoteServiceAdmin_stop(remote_service_admin_t *admin) {
celix_status_t status = CELIX_SUCCESS;
@@ -298,8 +366,16 @@ celix_status_t remoteServiceAdmin_stop(remote_service_admin_t *admin) {
for (i = 0; i < arrayList_size(exports); i++) {
export_registration_t *export = arrayList_get(exports, i);
if (export != NULL) {
- exportRegistration_stop(export);
- exportRegistration_destroy(export);
+ if (CELIX_RSA_USE_STOP_EXPORT_THREAD) {
+ celixThreadMutex_lock(&admin->stopExportsMutex);
+ exportRegistration_setActive(export, false);
+ celix_arrayList_add(admin->stopExports, export);
+ celixThreadCondition_broadcast(&admin->stopExportsCond);
+ celixThreadMutex_unlock(&admin->stopExportsMutex);
+ } else {
+ exportRegistration_stop(export);
+ exportRegistration_destroy(export);
+ }
}
}
arrayList_destroy(exports);
@@ -307,6 +383,8 @@ celix_status_t remoteServiceAdmin_stop(remote_service_admin_t *admin) {
hashMapIterator_destroy(iter);
celixThreadRwlock_unlock(&admin->exportedServicesLock);
+ remoteServiceAdmin_teardownStopExportsThread(admin);
+
celixThreadMutex_lock(&admin->importedServicesLock);
int i;
int size = arrayList_size(admin->importedServices);
@@ -555,8 +633,16 @@ celix_status_t remoteServiceAdmin_removeExportedService(remote_service_admin_t *
arrayList_destroy(exports);
}
- exportRegistration_close(registration);
- exportRegistration_destroy(registration);
+ if (CELIX_RSA_USE_STOP_EXPORT_THREAD) {
+ celixThreadMutex_lock(&admin->stopExportsMutex);
+ exportRegistration_setActive(registration, false);
+ celix_arrayList_add(admin->stopExports, registration);
+ celixThreadCondition_broadcast(&admin->stopExportsCond);
+ celixThreadMutex_unlock(&admin->stopExportsMutex);
+ } else {
+ exportRegistration_stop(registration);
+ exportRegistration_destroy(registration);
+ }
celixThreadRwlock_unlock(&admin->exportedServicesLock);