PengZheng commented on code in PR #476:
URL: https://github.com/apache/celix/pull/476#discussion_r1153339301
##########
libs/framework/src/bundle_archive.c:
##########
@@ -28,644 +28,415 @@
#include <dirent.h>
#include <unistd.h>
#include <string.h>
+#include <assert.h>
#include "celix_constants.h"
#include "celix_utils_api.h"
#include "linked_list_iterator.h"
#include "framework_private.h"
#include "celix_file_utils.h"
#include "bundle_revision_private.h"
+#include "celix_framework_utils_private.h"
+
+celix_status_t celix_bundleArchive_getLastModifiedInternal(bundle_archive_pt
archive, struct timespec *lastModified);
+
+/**
+ * The bundle archive which is used to store the bundle data and can be reused
when a framework is restarted.
+ * The lifecycle of a bundle archive is coupled to the lifecycle of the bundle
that is created from the archive.
+ *
+ * @note The bundle archive is thread safe.
+ */
struct bundleArchive {
+ //initialed during creation and immutable
celix_framework_t* fw;
long id;
- char * location;
- DIR *archiveRootDir;
- char * archiveRoot;
- linked_list_pt revisions;
- long refreshCount;
- time_t lastModified;
-
- bundle_state_e persistentState;
+ char *archiveRoot;
+ char *savedBundleStatePropertiesPath;
+ char* storeRoot;
+ char* resourceCacheRoot;
+ bool isSystemBundle;
+ char* bundleSymbolicName; //read from the manifest
+ char* bundleVersion; //read from the manifest
+
+ celix_thread_mutex_t lock; //protects below and saving of bundle state
properties
+ bundle_revision_t* revision; //the current revision
+ char* location;
};
-static celix_status_t bundleArchive_getRevisionLocation(bundle_archive_pt
archive, long revNr, char **revision_location);
-static celix_status_t bundleArchive_setRevisionLocation(bundle_archive_pt
archive, const char * location, long revNr);
-
-static celix_status_t bundleArchive_initialize(bundle_archive_pt archive);
-
-static celix_status_t
bundleArchive_createRevisionFromLocation(bundle_archive_pt archive, const char
*location, const char *inputFile, long revNr, bundle_revision_pt
*bundle_revision);
-static celix_status_t bundleArchive_reviseInternal(bundle_archive_pt archive,
bool isReload, long revNr, const char * location, const char *inputFile);
-
-static celix_status_t bundleArchive_readLastModified(bundle_archive_pt
archive, time_t *time);
-static celix_status_t bundleArchive_writeLastModified(bundle_archive_pt
archive);
-
-celix_status_t bundleArchive_createSystemBundleArchive(celix_framework_t* fw,
bundle_archive_pt *bundle_archive) {
- celix_status_t status = CELIX_SUCCESS;
- char *error = NULL;
- bundle_archive_pt archive = NULL;
-
- if (*bundle_archive != NULL) {
- status = CELIX_ILLEGAL_ARGUMENT;
- error = "Missing required arguments and/or incorrect values";
- } else {
- archive = (bundle_archive_pt) calloc(1,sizeof(*archive));
- if (archive == NULL) {
- status = CELIX_ENOMEM;
- } else {
- status = linkedList_create(&archive->revisions);
- if (status == CELIX_SUCCESS) {
- archive->fw = fw;
- archive->id = CELIX_FRAMEWORK_BUNDLE_ID;
- archive->location = strdup("System Bundle");
- archive->archiveRoot = NULL;
- archive->archiveRootDir = NULL;
- archive->refreshCount = -1;
- archive->persistentState =
CELIX_BUNDLE_STATE_UNKNOWN;
- time(&archive->lastModified);
-
- *bundle_archive = archive;
- }
- }
- }
-
- if(status != CELIX_SUCCESS && archive != NULL){
- bundleArchive_destroy(archive);
- }
-
- framework_logIfError(fw->logger, status, error, "Could not create
archive");
-
- return status;
+static void celix_bundleArchive_storeBundleStateProperties(bundle_archive_pt
archive) {
+ celix_properties_t* bundleStateProperties = celix_properties_create();
+ if (bundleStateProperties != NULL) {
+ celixThreadMutex_lock(&archive->lock);
+ //set/update bundle cache state properties
+ celix_properties_setLong(bundleStateProperties,
CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, archive->id);
+ celix_properties_set(bundleStateProperties,
CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, archive->location);
+ celix_properties_set(bundleStateProperties,
CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME,
+ archive->bundleSymbolicName);
+ celix_properties_set(bundleStateProperties,
CELIX_BUNDLE_ARCHIVE_VERSION_PROPERTY_NAME, archive->bundleVersion);
+
+ //save bundle cache state properties
+ celix_properties_store(bundleStateProperties,
archive->savedBundleStatePropertiesPath,
+ "Bundle State Properties");
+ celixThreadMutex_unlock(&archive->lock);
+ }
+ celix_properties_destroy(bundleStateProperties);
}
-celix_status_t bundleArchive_create(celix_framework_t* fw, const char
*archiveRoot, long id, const char * location, const char *inputFile,
bundle_archive_pt *bundle_archive) {
- celix_status_t status = CELIX_SUCCESS;
- char *error = NULL;
- bundle_archive_pt archive = NULL;
-
- if (*bundle_archive != NULL) {
- status = CELIX_ILLEGAL_ARGUMENT;
- error = "bundle_archive_pt must be NULL";
- } else {
- archive = (bundle_archive_pt) calloc(1,sizeof(*archive));
- if (archive == NULL) {
- status = CELIX_ENOMEM;
- } else {
- status = linkedList_create(&archive->revisions);
- if (status == CELIX_SUCCESS) {
- archive->fw = fw;
- archive->id = id;
- archive->location = strdup(location);
- archive->archiveRootDir = NULL;
- archive->archiveRoot = strdup(archiveRoot);
- archive->refreshCount = -1;
- time(&archive->lastModified);
-
- status = bundleArchive_initialize(archive);
- if (status == CELIX_SUCCESS) {
- status = bundleArchive_revise(archive,
location, inputFile);
-
- if (status == CELIX_SUCCESS) {
- *bundle_archive = archive;
- }
- else{
-
bundleArchive_closeAndDelete(archive);
- }
- }
- }
- }
- }
-
- if(status != CELIX_SUCCESS && archive != NULL){
- bundleArchive_destroy(archive);
- }
+celix_status_t celix_bundleArchive_extractBundle(
+ bundle_archive_t* archive,
+ const char* revisionRoot,
+ const char* bundleUrl,
+ const struct timespec* revisionModificationTime) {
+ celix_status_t status = CELIX_SUCCESS;
+ bool extractBundle = true;
+
+ //check if bundle location is newer than current revision
+ if (revisionModificationTime->tv_sec != 0 &&
revisionModificationTime->tv_nsec != 0) {
+ extractBundle =
celix_framework_utils_isBundleUrlNewerThan(archive->fw, bundleUrl,
revisionModificationTime);
+ }
+
+ if (!extractBundle) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_TRACE, "Bundle archive %s
is up to date, no need to extract bundle.", bundleUrl);
+ return status;
+ }
+
+ /*
+ * Note always remove the current revision dir. This is needed to remove
files that are not present
+ * in the new bundle zip, but it seems this is also needed to ensure that
the lib files get a new inode.
+ * If dlopen/dlsym is used with newer files, but with the same inode
already used in dlopen/dlsym this leads to
+ * segfaults.
+ */
+ const char* error;
+ status = celix_utils_deleteDirectory(revisionRoot, &error);
+ if (status != CELIX_SUCCESS) {
+ fw_logCode(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed
to remove existing bundle archive revision directory '%s': %s", revisionRoot,
error);
+ return status;
+ }
+
+ status = celix_framework_utils_extractBundle(archive->fw, bundleUrl,
revisionRoot);
+ if (status != CELIX_SUCCESS) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to extract bundle zip to revision directory.");
+ return status;
+ }
+ return status;
+}
- framework_logIfError(fw->logger, status, error, "Could not create
archive");
+/**
+ * Initialize archive by creating the bundle cache directory, optionally
extracting the bundle from the bundle file,
+ * reading the bundle state properties, reading the bundle manifest and
updating the bundle state properties.
+ */
+celix_status_t celix_bundleArchive_createCacheDirectory(bundle_archive_pt
archive, manifest_pt* manifestOut) {
+ if (celix_utils_fileExists(archive->archiveRoot)) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_TRACE, "Bundle archive
root for bundle id %li already exists.",
+ archive->id);
+ }
+
+ //create archive root
+ const char* errorStr = NULL;
+ celix_status_t status = celix_utils_createDirectory(archive->archiveRoot,
false, &errorStr);
+ if (status != CELIX_SUCCESS) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to create bundle root archive dir: %s", errorStr);
+ return status;
+ }
+
+ //create store directory
+ int rc = asprintf(&archive->storeRoot, "%s/%s", archive->archiveRoot,
CELIX_BUNDLE_ARCHIVE_STORE_DIRECTORY_NAME);
+ if (rc < 0) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to create bundle store dir.");
+ return CELIX_ENOMEM;
+ }
+ status = celix_utils_createDirectory(archive->storeRoot, false, &errorStr);
+ if (status != CELIX_SUCCESS) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to create bundle store dir: %s", errorStr);
+ return status;
+ }
+
+ //create bundle revision directory
+ rc = asprintf(&archive->resourceCacheRoot, "%s/%s", archive->archiveRoot,
CELIX_BUNDLE_ARCHIVE_RESOURCE_CACHE_NAME);
+ if (rc < 0) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to create bundle revision dir.");
+ return CELIX_ENOMEM;
+ }
+
+ status = celix_utils_createDirectory(archive->resourceCacheRoot, false,
&errorStr);
+ if (status != CELIX_SUCCESS) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to create bundle revision dir: %s", errorStr);
+ return status;
+ }
+
+ //get revision mod time;
+ struct timespec revisionMod;
+ status = celix_bundleArchive_getLastModifiedInternal(archive,
&revisionMod);
+ if (status != CELIX_SUCCESS) {
+ fw_logCode(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed
to get last modified time for bundle archive revision directory '%s'",
archive->resourceCacheRoot);
+ return status;
+ }
+
+ //extract bundle zip to revision directory
+ status = celix_bundleArchive_extractBundle(archive,
archive->resourceCacheRoot, archive->location, &revisionMod);
+ if (status != CELIX_SUCCESS) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Failed to extract bundle.");
+ return status;
+ }
+
+ //read manifest from extracted bundle zip
+ *manifestOut = NULL;
+ char pathBuffer[512];
+ char* manifestPath = celix_utils_writeOrCreateString(pathBuffer,
sizeof(pathBuffer), "%s/%s", archive->resourceCacheRoot,
CELIX_BUNDLE_MANIFEST_REL_PATH);
+ status = manifest_createFromFile(manifestPath, manifestOut);
+ celix_utils_freeStringIfNotEqual(pathBuffer, manifestPath);
+ if (status != CELIX_SUCCESS) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Cannot read manifest.");
+ return status;
+ }
+
+ //populate bundle symbolic name and version from manifest
+ archive->bundleSymbolicName =
celix_utils_strdup(manifest_getValue(*manifestOut,
OSGI_FRAMEWORK_BUNDLE_SYMBOLICNAME));
+ if (archive->bundleSymbolicName == NULL) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Cannot read bundle symbolic name.");
+ manifest_destroy(*manifestOut);
+ return CELIX_BUNDLE_EXCEPTION;
+ }
+ archive->bundleVersion =
celix_utils_strdup(manifest_getValue(*manifestOut,
OSGI_FRAMEWORK_BUNDLE_VERSION));
+ if (archive->bundleVersion == NULL) {
+ fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to
initialize archive. Cannot read bundle version.");
+ manifest_destroy(*manifestOut);
+ return CELIX_BUNDLE_EXCEPTION;
+ }
+
+ return status;
+}
+/**
+ * Create a bundle archive for the given root, id, location and revision nr.
+ *
+ * Also create the bundle cache dir and if will reuse a existing bundle
resource cache dir if the provided
+ * bundle zip location is older then the existing bundle resource cache dir.
+ */
+celix_status_t celix_bundleArchive_createArchiveInternal(celix_framework_t*
fw, const char* archiveRoot, long id, const char *location, bundle_archive_pt*
bundle_archive) {
+ celix_status_t status = CELIX_SUCCESS;
+ bundle_archive_pt archive = calloc(1, sizeof(*archive));
+
+ if (archive) {
+ archive->fw = fw;
+ archive->id = id;
+ archive->isSystemBundle = id == CELIX_FRAMEWORK_BUNDLE_ID;
+ celixThreadMutex_create(&archive->lock, NULL);
+ }
+
+ if (archive == NULL) {
+ status = CELIX_ENOMEM;
+ fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not
create archive. Out of memory.");
+ bundleArchive_destroy(archive);
+ return status;
+ }
+
+ int rc;
+ if (archive->isSystemBundle) {
+ archive->resourceCacheRoot = getcwd(NULL, 0);
+ archive->storeRoot = getcwd(NULL, 0);
+ } else {
+ // assert(location != NULL)
+ archive->location = celix_utils_strdup(location);
+ archive->archiveRoot = celix_utils_strdup(archiveRoot);
+ rc = asprintf(&archive->savedBundleStatePropertiesPath, "%s/%s",
archiveRoot,
+ CELIX_BUNDLE_ARCHIVE_STATE_PROPERTIES_FILE_NAME);
+ if (rc < 0 || archive->location == NULL ||
archive->savedBundleStatePropertiesPath == NULL
+ || archive->archiveRoot == NULL) {
+ status = CELIX_ENOMEM;
+ fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not
create archive. Out of memory.");
+ bundleArchive_destroy(archive);
+ return status;
+ }
+ }
+
+ manifest_pt manifest = NULL;
+ if (archive->isSystemBundle) {
+ status = manifest_create(&manifest);
+ } else {
+ status = celix_bundleArchive_createCacheDirectory(archive, &manifest);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not
create archive. Failed to initialize archive or create manifest.");
+ bundleArchive_destroy(archive);
+ return status;
+ }
+
+ if (archive->isSystemBundle) {
+ status = bundleRevision_create(fw, archive->archiveRoot, NULL,
manifest, &archive->revision);
+ } else {
+ status = bundleRevision_create(fw, archive->archiveRoot,
archive->location, manifest, &archive->revision);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not
create archive. Could not create bundle revision.");
+ bundleArchive_destroy(archive);
+ return status;
+ }
+
+ if (!archive->isSystemBundle) {
+ celix_bundleArchive_storeBundleStateProperties(archive);
+ }
+
+ *bundle_archive = archive;
+ return status;
+}
- return status;
+celix_status_t bundleArchive_create(celix_framework_t* fw, const char
*archiveRoot, long id, const char *location, bundle_archive_pt *bundle_archive)
{
+ return celix_bundleArchive_createArchiveInternal(fw, archiveRoot, id,
location, bundle_archive);
}
celix_status_t bundleArchive_destroy(bundle_archive_pt archive) {
if (archive != NULL) {
- if (archive->revisions != NULL) {
- linked_list_iterator_pt iter =
linkedListIterator_create(archive->revisions, 0);
- while(linkedListIterator_hasNext(iter)) {
- bundle_revision_pt rev =
linkedListIterator_next(iter);
- bundleRevision_destroy(rev);
- }
- linkedListIterator_destroy(iter);
- linkedList_destroy(archive->revisions);
- }
- if (archive->archiveRoot != NULL) {
- free(archive->archiveRoot);
- }
- if (archive->location != NULL) {
- free(archive->location);
- }
-
- free(archive);
- archive = NULL;
+ free(archive->location);
+ free(archive->savedBundleStatePropertiesPath);
+ free(archive->archiveRoot);
+ free(archive->resourceCacheRoot);
+ free(archive->storeRoot);
+ free(archive->bundleSymbolicName);
+ free(archive->bundleVersion);
+ bundleRevision_destroy(archive->revision);
+ celixThreadMutex_destroy(&archive->lock);
+ free(archive);
}
return CELIX_SUCCESS;
}
-celix_status_t bundleArchive_recreate(celix_framework_t* fw, const char *
archiveRoot, bundle_archive_pt *bundle_archive) {
- celix_status_t status = CELIX_SUCCESS;
-
- bundle_archive_pt archive = NULL;
-
- archive = (bundle_archive_pt) calloc(1,sizeof(*archive));
- if (archive == NULL) {
- status = CELIX_ENOMEM;
- } else {
- status = linkedList_create(&archive->revisions);
- if (status == CELIX_SUCCESS) {
- archive->fw = fw;
- archive->archiveRoot = strdup(archiveRoot);
- archive->archiveRootDir = NULL;
- archive->id = -1;
- archive->persistentState = -1;
- archive->location = NULL;
- archive->refreshCount = -1;
- archive->lastModified = (time_t) NULL;
-
- archive->archiveRootDir = opendir(archiveRoot);
- if (archive->archiveRootDir == NULL) {
- status = CELIX_FRAMEWORK_EXCEPTION;
- } else {
-
- long idx = 0;
- long highestId = -1;
- char *location = NULL;
-
- struct dirent *dent = NULL;
- struct stat st;
-
- errno = 0;
- dent = readdir(archive->archiveRootDir);
- while (errno == 0 && dent != NULL) {
- char subdir[512];
- snprintf(subdir, 512, "%s/%s",
archiveRoot, dent->d_name);
- int rv = stat(subdir, &st);
- if (rv == 0 && S_ISDIR(st.st_mode) &&
(strncmp(dent->d_name, "version", 7) == 0)) {
- sscanf(dent->d_name,
"version%*d.%ld", &idx);
- if (idx > highestId) {
- highestId = idx;
- }
- }
- errno = 0;
- dent = readdir(archive->archiveRootDir);
- }
-
- status = CELIX_DO_IF(status,
bundleArchive_getRevisionLocation(archive, 0, &location));
- status = CELIX_DO_IF(status,
bundleArchive_reviseInternal(archive, true, highestId, location, NULL));
- if (location) {
- free(location);
- }
- if (status == CELIX_SUCCESS) {
- *bundle_archive = archive;
- }
- closedir(archive->archiveRootDir);
- }
- }
- }
-
- if(status != CELIX_SUCCESS && archive != NULL){
- bundleArchive_destroy(archive);
- }
-
- framework_logIfError(fw->logger, status, NULL, "Could not create
archive");
-
- return status;
-}
-
celix_status_t bundleArchive_getId(bundle_archive_pt archive, long *id) {
- celix_status_t status = CELIX_SUCCESS;
-
- if (archive->id < 0) {
- FILE *bundleIdFile;
- char id[256];
- char bundleId[512];
- snprintf(bundleId, sizeof(bundleId), "%s/bundle.id",
archive->archiveRoot);
-
- bundleIdFile = fopen(bundleId, "r");
- if(bundleIdFile!=NULL){
- fgets(id, sizeof(id), bundleIdFile);
- fclose(bundleIdFile);
- sscanf(id, "%ld", &archive->id);
- }
- else{
- status = CELIX_FILE_IO_EXCEPTION;
- }
- }
-
- if (status == CELIX_SUCCESS) {
- *id = archive->id;
- }
+ *id = archive->id;
+ return CELIX_SUCCESS;
+}
- framework_logIfError(archive->fw->logger, status, NULL, "Could not get
archive id");
+long celix_bundleArchive_getId(bundle_archive_pt archive) {
+ return archive->id;
+}
- return status;
+const char* celix_bundleArchive_getSymbolicName(bundle_archive_pt archive) {
+ return archive->bundleSymbolicName;
}
celix_status_t bundleArchive_getLocation(bundle_archive_pt archive, const char
**location) {
- celix_status_t status = CELIX_SUCCESS;
- if (archive->location == NULL) {
- FILE *bundleLocationFile;
- char bundleLocation[512];
- char loc[256];
-
- snprintf(bundleLocation, sizeof(bundleLocation),
"%s/bundle.location", archive->archiveRoot);
-
- bundleLocationFile = fopen(bundleLocation, "r");
- if(bundleLocationFile!=NULL){
- fgets(loc, sizeof(loc), bundleLocationFile);
- fclose(bundleLocationFile);
- archive->location = strdup(loc);
- }
- else{
- status = CELIX_FILE_IO_EXCEPTION;
- }
- }
-
- if (status == CELIX_SUCCESS) {
- *location = archive->location;
- }
-
- framework_logIfError(archive->fw->logger, status, NULL, "Could not get
archive location");
-
- return status;
+ *location = archive->location;
+ return CELIX_SUCCESS;
}
celix_status_t bundleArchive_getArchiveRoot(bundle_archive_pt archive, const
char **archiveRoot) {
- *archiveRoot = archive->archiveRoot;
- return CELIX_SUCCESS;
+ *archiveRoot = archive->archiveRoot;
+ return CELIX_SUCCESS;
}
celix_status_t bundleArchive_getCurrentRevisionNumber(bundle_archive_pt
archive, long *revisionNumber) {
- celix_status_t status = CELIX_SUCCESS;
- bundle_revision_pt revision;
- *revisionNumber = -1;
-
- status = CELIX_DO_IF(status, bundleArchive_getCurrentRevision(archive,
&revision));
- status = CELIX_DO_IF(status, bundleRevision_getNumber(revision,
revisionNumber));
-
- framework_logIfError(archive->fw->logger, status, NULL, "Could not get
current revision number");
-
- return status;
+ *revisionNumber = 1; //NOTE bundle revision is deprecated
+ return CELIX_SUCCESS;
}
celix_status_t bundleArchive_getCurrentRevision(bundle_archive_pt archive,
bundle_revision_pt *revision) {
- *revision = linkedList_isEmpty(archive->revisions) ? NULL :
linkedList_getLast(archive->revisions);
- return CELIX_SUCCESS;
+ *revision = archive->revision;
Review Comment:
It's more difficult to fix this use-after-free without changing interface.
Consider when it returns, `archive->revision` could be freed by
`bundleArchive_revise` but is stilled used by the caller.
This remark also applies to `bundleArchive_getLocation`
--
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
