PengZheng commented on code in PR #476: URL: https://github.com/apache/celix/pull/476#discussion_r1140162136
########## libs/framework/src/bundle_archive.c: ########## @@ -28,644 +28,451 @@ #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; + bool isSystemBundle; + char* bundleSymbolicName; //read from the manifest + char* bundleVersion; //read from the manifest -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); + celix_thread_mutex_t lock; //protects below and saving of bundle state properties -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); + char* location; + char* currentRevisionRoot; + celix_array_list_t* revisions; //list of bundle_revision_t* -static celix_status_t bundleArchive_readLastModified(bundle_archive_pt archive, time_t *time); -static celix_status_t bundleArchive_writeLastModified(bundle_archive_pt archive); + //bundle cache state info + long revisionNr; -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"); + //bundle cache state in properties form + celix_properties_t* bundleStateProperties; +}; - return status; +static void celix_bundleArchive_updateAndStoreBundleStateProperties(bundle_archive_pt archive) { + celixThreadMutex_lock(&archive->lock); + //set/update bundle cache state properties + celix_properties_setLong(archive->bundleStateProperties, CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, archive->id); + celix_properties_set(archive->bundleStateProperties, CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, archive->location); + celix_properties_set(archive->bundleStateProperties, CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME, archive->bundleSymbolicName); + celix_properties_set(archive->bundleStateProperties, CELIX_BUNDLE_ARCHIVE_VERSION_PROPERTY_NAME, archive->bundleVersion); + celix_properties_setLong(archive->bundleStateProperties, CELIX_BUNDLE_ARCHIVE_REVISION_PROPERTY_NAME, archive->revisionNr); + + //save bundle cache state properties + celix_properties_store(archive->bundleStateProperties, archive->savedBundleStatePropertiesPath, "Bundle State Properties"); + celixThreadMutex_unlock(&archive->lock); } -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 (celix_utils_fileExists(revisionRoot)) { + 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->currentRevisionRoot, CELIX_BUNDLE_ARCHIVE_REVISION_DIRECTORY_NAME_FORMAT, archive->archiveRoot, archive->revisionNr); + 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->currentRevisionRoot, 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->currentRevisionRoot); + return status; + } + + //extract bundle zip to revision directory + status = celix_bundleArchive_extractBundle(archive, archive->currentRevisionRoot, 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->currentRevisionRoot, 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."); + manifest_destroy(*manifestOut); Review Comment: ```suggestion ``` `manifest_createFromFile` never leaks manifest in error condition. -- 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