This is an automated email from the ASF dual-hosted git repository.
pengzheng pushed a commit to branch
hotfix/595-use-after-free-caused-by-cancelled-tracker
in repository https://gitbox.apache.org/repos/asf/celix.git
The following commit(s) were added to
refs/heads/hotfix/595-use-after-free-caused-by-cancelled-tracker by this push:
new 7c65bfce [#595] fix use-after-free caused by cancelled service tracker.
7c65bfce is described below
commit 7c65bfce2792d3ec03d980ec97ef0527200d4a50
Author: PengZheng <[email protected]>
AuthorDate: Wed Jul 26 10:36:50 2023 +0800
[#595] fix use-after-free caused by cancelled service tracker.
---
.../src/discovery_zeroconf_watcher.c | 3 +-
.../framework/include_deprecated/service_tracker.h | 9 +++++
libs/framework/src/bundle_context.c | 27 ++++++-------
libs/framework/src/service_tracker.c | 45 +++++++++++++++-------
4 files changed, 52 insertions(+), 32 deletions(-)
diff --git
a/bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_watcher.c
b/bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_watcher.c
index 1fcf997c..b62c0301 100644
---
a/bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_watcher.c
+++
b/bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_watcher.c
@@ -153,8 +153,7 @@ celix_status_t
discoveryZeroconfWatcher_create(celix_bundle_context_t *ctx, celi
*watcherOut = watcher;
return CELIX_SUCCESS;
thread_err:
- celix_bundleContext_stopTrackerAsync(ctx, watcher->epListenerTrkId, NULL,
NULL);
- celix_bundleContext_waitForAsyncStopTracker(ctx, watcher->epListenerTrkId);
+ celix_bundleContext_stopTracker(ctx, watcher->epListenerTrkId);
epl_tracker_err:
celix_longHashMap_destroy(watcher->epls);
celix_stringHashMap_destroy(watcher->watchedServices);
diff --git a/libs/framework/include_deprecated/service_tracker.h
b/libs/framework/include_deprecated/service_tracker.h
index a92b290d..d65834f3 100644
--- a/libs/framework/include_deprecated/service_tracker.h
+++ b/libs/framework/include_deprecated/service_tracker.h
@@ -83,6 +83,15 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_service_tracker_t*
celix_serviceTracker_
);
+/**
+ * Creates a closed service tracker.
+ * It is similar to serviceTracker_create.
+ */
+CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_service_tracker_t*
celix_serviceTracker_createClosedWithOptions(
+ celix_bundle_context_t *ctx,
+ const celix_service_tracking_options_t *opts
+);
+
/**
* Creates and starts (open) a service tracker.
* Note that is different from the serviceTracker_create function, because is
also starts the service tracker
diff --git a/libs/framework/src/bundle_context.c
b/libs/framework/src/bundle_context.c
index 1d78780b..05541e16 100644
--- a/libs/framework/src/bundle_context.c
+++ b/libs/framework/src/bundle_context.c
@@ -1235,25 +1235,20 @@ static void
celix_bundleContext_createTrackerOnEventLoop(void *data) {
assert(celix_framework_isCurrentThreadTheEventLoop(entry->ctx->framework));
celixThreadMutex_lock(&entry->ctx->mutex);
bool cancelled = entry->cancelled;
- celixThreadMutex_unlock(&entry->ctx->mutex);
if (cancelled) {
fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Creating
of service tracker was cancelled. trk id = %li, svc name tracked = %s",
entry->trackerId, entry->opts.filter.serviceName);
+ celixThreadMutex_unlock(&entry->ctx->mutex);
+ return;
+ }
+ celix_service_tracker_t *tracker =
celix_serviceTracker_createClosedWithOptions(entry->ctx, &entry->opts);
+ if (tracker == NULL) {
+ fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot
create tracker for bnd %s (%li)",
celix_bundle_getSymbolicName(entry->ctx->bundle),
celix_bundle_getId(entry->ctx->bundle));
} else {
- celix_service_tracker_t *tracker =
celix_serviceTracker_createWithOptions(entry->ctx, &entry->opts);
- if (tracker != NULL) {
- celixThreadMutex_lock(&entry->ctx->mutex);
- // double-check is necessary to eliminate first-check-then-do race
condition
- if(!entry->cancelled) {
- entry->tracker = tracker;
- }
- celixThreadMutex_unlock(&entry->ctx->mutex);
- if(entry->tracker == NULL) {
- assert(entry->cancelled);
- celix_serviceTracker_destroy(tracker);
- }
- } else {
- fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR,
"Cannot create tracker for bnd %s (%li)",
celix_bundle_getSymbolicName(entry->ctx->bundle),
celix_bundle_getId(entry->ctx->bundle));
- }
+ entry->tracker = tracker;
+ }
+ celixThreadMutex_unlock(&entry->ctx->mutex);
+ if (tracker) {
+ serviceTracker_open(tracker);
}
}
diff --git a/libs/framework/src/service_tracker.c
b/libs/framework/src/service_tracker.c
index 750f336b..76e69565 100644
--- a/libs/framework/src/service_tracker.c
+++ b/libs/framework/src/service_tracker.c
@@ -632,28 +632,35 @@ celix_service_tracker_t* celix_serviceTracker_create(
return celix_serviceTracker_createWithOptions(ctx, &opts);
}
-celix_service_tracker_t* celix_serviceTracker_createWithOptions(
- bundle_context_t *ctx,
- const celix_service_tracking_options_t *opts
-) {
- celix_service_tracker_t *tracker = NULL;
+celix_service_tracker_t*
celix_serviceTracker_createClosedWithOptions(celix_bundle_context_t* ctx,
+ const
celix_service_tracking_options_t* opts) {
+ celix_service_tracker_t* tracker = NULL;
const char* serviceName = NULL;
- char *filter = NULL;
+ char* filter = NULL;
assert(ctx != NULL);
assert(opts != NULL);
serviceName = opts->filter.serviceName == NULL ? "*" :
opts->filter.serviceName;
- filter = celix_serviceRegistry_createFilterFor(ctx->framework->registry,
opts->filter.serviceName, opts->filter.versionRange, opts->filter.filter);
- if(filter == NULL) {
- celix_framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR,
__FUNCTION__, __BASE_FILE__, __LINE__,
+ filter = celix_serviceRegistry_createFilterFor(
+ ctx->framework->registry, opts->filter.serviceName,
opts->filter.versionRange, opts->filter.filter);
+ if (filter == NULL) {
+ celix_framework_log(ctx->framework->logger,
+ CELIX_LOG_LEVEL_ERROR,
+ __FUNCTION__,
+ __BASE_FILE__,
+ __LINE__,
"Error cannot create filter.");
return NULL;
}
tracker = calloc(1, sizeof(*tracker));
- if(tracker == NULL) {
+ if (tracker == NULL) {
free(filter);
- celix_framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR,
__FUNCTION__, __BASE_FILE__, __LINE__,
+ celix_framework_log(ctx->framework->logger,
+ CELIX_LOG_LEVEL_ERROR,
+ __FUNCTION__,
+ __BASE_FILE__,
+ __LINE__,
"No memory for tracker.");
- return NULL;
+ return NULL;
}
tracker->context = ctx;
@@ -661,7 +668,7 @@ celix_service_tracker_t*
celix_serviceTracker_createWithOptions(
tracker->filter = filter;
tracker->state = CELIX_SERVICE_TRACKER_CLOSED;
- //setting callbacks
+ // setting callbacks
tracker->callbackHandle = opts->callbackHandle;
tracker->set = opts->set;
tracker->add = opts->add;
@@ -684,8 +691,18 @@ celix_service_tracker_t*
celix_serviceTracker_createWithOptions(
tracker->currentHighestServiceId = -1;
tracker->listener.handle = tracker;
- tracker->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
+ tracker->listener.serviceChanged = (void*)serviceTracker_serviceChanged;
+ return tracker;
+}
+celix_service_tracker_t* celix_serviceTracker_createWithOptions(
+ bundle_context_t *ctx,
+ const celix_service_tracking_options_t *opts
+) {
+ celix_service_tracker_t *tracker =
celix_serviceTracker_createClosedWithOptions(ctx, opts);
+ if(tracker == NULL) {
+ return NULL;
+ }
serviceTracker_open(tracker);
return tracker;
}
