You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2020/07/16 14:00:53 UTC

[GitHub] [celix] pjw commented on a change in pull request #259: Feature/cxx

pjw commented on a change in pull request #259:
URL: https://github.com/apache/celix/pull/259#discussion_r455760607



##########
File path: misc/experimental/cxx_framework/examples/Introduction/src/UseService.cc
##########
@@ -0,0 +1,44 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <iostream>
+#include <chrono>
+#include "celix/Api.h"
+#include "examples/IHelloWorld.h"
+
+namespace {
+
+    class UseServiceActivator : public celix::IBundleActivator {
+    public:
+        explicit UseServiceActivator(const std::shared_ptr<celix::BundleContext>& ctx) {
+            ctx->buildUseService<examples::IHelloWorld>()
+                    .setFilter("(meta.info=value)")
+                    .waitFor(std::chrono::seconds{1})

Review comment:
       When would you ever want to wait before using the service? I assume the service framework guaranties that the service will only be provided once it is fully operational?

##########
File path: misc/experimental/cxx_framework/bundles/Shell/src/commands.h
##########
@@ -0,0 +1,45 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#ifndef CELIX_COMMANDS_H
+#define CELIX_COMMANDS_H
+
+#include "celix/Api.h"
+
+namespace celix {
+namespace impl {
+    celix::ServiceRegistration registerLb(const std::shared_ptr<celix::BundleContext>& ctx);

Review comment:
       Why the use of a const shared_ptr? Would a const ref suffice?

##########
File path: misc/experimental/cxx_framework/examples/Introduction/src/ComponentExample.cc
##########
@@ -0,0 +1,65 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <iostream>
+#include <chrono>
+#include "celix/Api.h"
+#include "celix/IShellCommand.h"
+#include "examples/IHelloWorld.h"
+
+namespace {
+    class ComponentExample : public celix::IShellCommand {
+    public:
+        void setHelloWorld(const std::shared_ptr<examples::IHelloWorld>& svc) {

Review comment:
       Why a const ref to a shared_ptr? Why not a shared_ptr by value?

##########
File path: misc/experimental/cxx_framework/examples/Introduction/src/TrackExample.cc
##########
@@ -0,0 +1,51 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <iostream>
+#include <vector>
+#include "celix/Api.h"
+#include "examples/IHelloWorld.h"
+
+namespace {
+
+    class TrackExampleActivator : public celix::IBundleActivator {
+    public:
+        explicit TrackExampleActivator(const std::shared_ptr<celix::BundleContext>& ctx) {
+            celix::ServiceTracker tracker = ctx->buildServiceTracker<examples::IHelloWorld>()
+                    .setCallback([this](auto& svc) {
+                        std::lock_guard<std::mutex> lck{mutex};
+                        helloWorldSvc = svc;
+                        std::cout << "updated hello world service: " << (svc ? svc->sayHello() : "nullptr") << std::endl;
+                    })
+                    .build();
+            trackers.emplace_back(std::move(tracker));
+        }
+    private:
+        std::mutex mutex{}; //protect belows
+        std::shared_ptr<examples::IHelloWorld> helloWorldSvc{nullptr};
+        std::vector<celix::ServiceTracker> trackers{};

Review comment:
       Why a vector of service trackers?

##########
File path: misc/experimental/cxx_framework/examples/Introduction/src/ComponentExample.cc
##########
@@ -0,0 +1,65 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <iostream>
+#include <chrono>
+#include "celix/Api.h"
+#include "celix/IShellCommand.h"
+#include "examples/IHelloWorld.h"
+
+namespace {
+    class ComponentExample : public celix::IShellCommand {
+    public:
+        void setHelloWorld(const std::shared_ptr<examples::IHelloWorld>& svc) {
+            std::lock_guard<std::mutex> lck{mutex};
+            helloWorld = svc;
+        }
+        void executeCommand(const std::string&, const std::vector<std::string>&, std::ostream &out, std::ostream &/*err*/) override {
+            std::lock_guard<std::mutex> lck{mutex};
+            if (helloWorld) {
+                out << helloWorld->sayHello() << std::endl;
+            } else {
+                out << "IHelloWorld service not available" << std::endl;
+            }
+        }
+    private:
+        std::mutex mutex{};
+        std::shared_ptr<examples::IHelloWorld> helloWorld{nullptr};

Review comment:
       Storing a shared_ptr to the service suggests that the helloworld service is assumed to be operational for at least the lifetime of ComponentExample. I'm not sure that this is always the case. What if helloworld service is no longer operational, possibly due to some missing dependency, but ComponentExample is? How can helloworld 'deregister' itself from ComponentExample?

##########
File path: misc/experimental/cxx_framework/libs/Registry/include/celix/ServiceRegistrationBuilder.h
##########
@@ -0,0 +1,297 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#pragma once
+
+#include "celix/ServiceRegistry.h"
+
+namespace celix {
+
+    template<typename I>
+    class ServiceRegistrationBuilder {
+    public:
+        ServiceRegistrationBuilder(std::shared_ptr<celix::IResourceBundle> owner, std::shared_ptr<celix::ServiceRegistry> registry);
+        ServiceRegistrationBuilder(ServiceRegistrationBuilder<I> &&);
+        ServiceRegistrationBuilder<I>& operator=(ServiceRegistrationBuilder<I>&&);
+
+        ServiceRegistrationBuilder<I>& setService(std::shared_ptr<I> svc);
+        ServiceRegistrationBuilder<I>& setService(I *svc); //TODO do we really want to support naked pointers?

Review comment:
       There is nothing wrong with a naked pointer. Smart pointers are there to help with ownership. 
   
   If `setService()` acquires ownership of `svc` then a `unique_ptr` by value is perhaps the most clear interface. The supplier of `svc` needs to move the value into the `setService()` call, which is a clear indication of transferring ownership. Inside `setService()` the service can be moved into its final resting place.
   
   When `setService()` is not the sole owner of the service but merely wants to participate in ownership, then a `shared_ptr` is the way to go.
   
   When `setService()` is merely an observer (only for the duration of `setService()`, which the name does _not_ imply) then a naked pointer or reference, perhaps const,  is the way to go.

##########
File path: misc/experimental/cxx_framework/libs/Framework/src/Bundle.cc
##########
@@ -0,0 +1,95 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <string>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "Bundle.h"
+
+celix::Bundle::Bundle(long _bndId, const std::string &cacheDir, celix::Framework *_fw, celix::Properties _manifest) :
+        bndId{_bndId}, fw{_fw}, bndManifest{std::move(_manifest)},
+        bundleCache{cacheDir + "/bundle" + std::to_string(_bndId)},
+        bndState{BundleState::INSTALLED} {
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_SYMBOLIC_NAME));
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_NAME));
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_GROUP));
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_VERSION));
+    bndState.store(BundleState::INSTALLED, std::memory_order_release);
+}
+
+
+bool celix::Bundle::hasCacheEntry(const std::string &path) const {
+    auto abs = absPathForCacheEntry(path);
+    struct stat st{};
+    bool exists = stat(abs.c_str(), &st) == 0;
+    return exists;
+}
+
+bool celix::Bundle::isCacheEntryDir(const std::string &) const { return false; } //TODO
+
+bool celix::Bundle::isCacheEntryFile(const std::string &) const { return false; } //TODO
+
+std::vector <std::string> celix::Bundle::readCacheDir(const std::string &) const { //TODO
+    return std::vector < std::string > {};
+}
+
+const std::string& celix::Bundle::cacheRoot() const {
+    return bundleCache;
+}
+
+std::string celix::Bundle::absPathForCacheEntry(const std::string &entryPath) const {
+    return bundleCache + "/" + entryPath;
+}
+
+const std::string &celix::Bundle::name() const {
+    return bndManifest[celix::MANIFEST_BUNDLE_NAME];
+}
+
+const std::string &celix::Bundle::group() const {
+    return bndManifest[celix::MANIFEST_BUNDLE_GROUP];
+}
+
+const std::string &celix::Bundle::version() const {
+    return bndManifest[celix::MANIFEST_BUNDLE_VERSION];
+}
+
+const celix::Properties &celix::Bundle::manifest() const {
+    return bndManifest;
+}
+
+bool celix::Bundle::isValid() const {
+    return bndId >= 0;
+}
+
+celix::Framework &celix::Bundle::framework() const {
+    return *fw;
+}
+
+celix::BundleState celix::Bundle::state() const {
+    return bndState.load(std::memory_order_acquire);

Review comment:
       Playing with `std::memory_order` is like juggling with razor blades. Is atomicity required at this point?

##########
File path: misc/experimental/cxx_framework/libs/Registry/include/celix/ServiceRegistry.h
##########
@@ -0,0 +1,562 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#pragma once
+
+#include <utility>
+#include <vector>
+#include <functional>
+#include <memory>
+#include <string_view>
+
+#include "celix/Constants.h"
+#include "celix/Properties.h"
+#include "celix/Filter.h"
+#include "celix/Utils.h"
+#include "celix/IResourceBundle.h"
+#include "celix/IServiceFactory.h"
+#include "celix/ServiceTracker.h"
+#include "celix/ServiceRegistration.h"
+
+namespace celix {
+
+    template<typename I>
+    struct UseServiceOptions {
+        int limit{1}; //the limit for the services to be found. 0 -> unlimited
+        celix::Filter filter{};
+        long targetServiceId{-1L}; //note -1 means not targeting a specific service id.

Review comment:
       Could this be part of the functionality provided by `celix::Filter`?

##########
File path: misc/experimental/cxx_framework/libs/Framework/src/FrameworkImpl.cc
##########
@@ -0,0 +1,427 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include "celix/Framework.h"
+
+#include <memory>
+#include <utility>
+#include <map>
+#include <mutex>
+#include <vector>
+#include <unordered_set>
+#include <future>
+#include <algorithm>
+#include <climits>
+
+#include <uuid/uuid.h>
+
+#include "BundleContextImpl.h"
+#include "BundleController.h"
+
+#define LOGGER celix::getLogger("celix::Framework")
+
+extern bool extractBundle(const char *bundleZip, const char *targetPath); //FROM miniunz.c
+
+
+struct StaticBundleEntry {
+    const std::string symbolicName;
+    const celix::Properties manifest;
+    const std::function<celix::IBundleActivator *(std::shared_ptr<celix::BundleContext>)> activatorFactory;
+    const uint8_t *resourcesZip;
+    const size_t resourcesZipLen;
+};
+
+static struct {
+    std::mutex mutex{};
+    std::vector<StaticBundleEntry> bundles{};
+    std::unordered_set<celix::Framework*> frameworks{};
+} staticRegistry{};
+
+
+static void registerFramework(celix::Framework* fw);
+
+static void unregisterFramework(celix::Framework *fw);
+
+namespace celix::impl {
+    //some utils functions
+    static celix::Properties createFwManifest() {
+        celix::Properties m{};
+        m[celix::MANIFEST_BUNDLE_SYMBOLIC_NAME] = "celix::Framework";
+        m[celix::MANIFEST_BUNDLE_NAME] = "Celix Framework";
+        m[celix::MANIFEST_BUNDLE_GROUP] = "celix";
+        m[celix::MANIFEST_BUNDLE_VERSION] = "3.0.0";
+        return m;
+    }
+
+    static std::string genUUIDString() {
+        char uuidStr[37];
+        uuid_t uuid;
+        uuid_generate(uuid);
+        uuid_unparse(uuid, uuidStr);
+        return std::string{uuidStr};
+    }
+
+    static std::string createCwdString() {
+        char workdir[PATH_MAX];
+        if (getcwd(workdir, sizeof(workdir)) != nullptr) {
+            return std::string{workdir};
+        } else {
+            return std::string{};
+        }
+    }
+
+    static std::string genFwCacheDir(const celix::Properties &/*fwConfig*/) {
+        //TODO make configure-able
+        return createCwdString() + "/.cache";
+    }
+
+    class FrameworkImpl : public celix::Framework {
+    public:
+        FrameworkImpl(celix::Properties _config) :
+                config{std::move(_config)},
+                bndManifest{createFwManifest()},
+                cwd{createCwdString()},
+                fwUUID{genUUIDString()},
+                fwCacheDir{genFwCacheDir(config)} {
+            LOGGER->trace("Celix Framework {} created", shortUUID());
+        }
+
+        ~FrameworkImpl() override {
+            LOGGER->trace("Celix Framework {} destroyed", shortUUID());
+        }
+
+        std::vector<long> listBundles(bool includeFrameworkBundle) const override {
+            std::vector<long> result{};
+            std::lock_guard<std::mutex> lock{bundles.mutex};
+            for (auto &entry : bundles.entries) {
+                if (entry.second->bundle()->id() == FRAMEWORK_BUNDLE_ID) {
+                    if (includeFrameworkBundle) {
+                        result.push_back(entry.first);
+                    }
+                } else {
+                    result.push_back(entry.first);
+                }
+            }
+            std::sort(result.begin(), result.end());//ensure that the bundles are order by bndId -> i.e. time of install
+            return result;
+        }
+
+        virtual long installBundle(
+                std::string symbolicName,
+                std::function<celix::IBundleActivator *(std::shared_ptr<celix::BundleContext>)> actFactory,
+                celix::Properties manifest,
+                bool autoStart,
+                const uint8_t *resourcesZip,
+                size_t resourcesZipLen) override {
+            //TODO on separate thread ?? specific bundle resolve thread ??
+            long bndId = -1L;
+            if (symbolicName.empty()) {
+                LOGGER->error("Cannot install bundle with an empty symbolic name!");
+                return bndId;
+            }
+
+            std::shared_ptr<celix::BundleController> bndController{nullptr};
+            {
+                manifest[celix::MANIFEST_BUNDLE_SYMBOLIC_NAME] = symbolicName;
+                if (manifest.find(celix::MANIFEST_BUNDLE_NAME) == manifest.end()) {
+                    manifest[celix::MANIFEST_BUNDLE_NAME] = symbolicName;
+                }
+                if (manifest.find(celix::MANIFEST_BUNDLE_VERSION) == manifest.end()) {
+                    manifest[celix::MANIFEST_BUNDLE_VERSION] = "0.0.0";
+                }
+                if (manifest.find(celix::MANIFEST_BUNDLE_GROUP) == manifest.end()) {
+                    manifest[celix::MANIFEST_BUNDLE_GROUP] = "";
+                }
+
+                std::lock_guard<std::mutex> lck{bundles.mutex};
+                bndId = bundles.nextBundleId++;
+                auto bnd = std::make_shared<celix::Bundle>(bndId, this->cacheDir(), this /*TODO improve*/,
+                                                           std::move(manifest));
+                auto ctx = std::make_shared<celix::impl::BundleContextImpl>(bnd);
+                bndController = std::make_shared<celix::BundleController>(std::move(actFactory), bnd, ctx, resourcesZip,
+                                                                          resourcesZipLen);
+                bundles.entries.emplace(std::piecewise_construct,
+                                        std::forward_as_tuple(bndId),
+                                        std::forward_as_tuple(bndController));
+
+                //TODO increase bnd entry usage
+            }
+
+            if (bndController) {
+                if (autoStart) {
+                    bool successful = bndController->transitionTo(BundleState::ACTIVE);
+                    if (!successful) {
+                        //TODO move to cc file and add LOGGER
+                        //LOG(WARNING) << "Cannot start bundle " << bndController->bundle()->symbolicName() << std::endl;
+                    }
+                }
+            }
+
+            return bndId;
+        }
+
+        bool startBundle(long bndId) override {
+            if (bndId == FRAMEWORK_BUNDLE_ID) {
+                return startFramework();
+            } else {
+                return transitionBundleTo(bndId, BundleState::ACTIVE);
+            }
+        }
+
+        bool stopBundle(long bndId) override {
+            if (bndId == FRAMEWORK_BUNDLE_ID) {
+                return stopFramework();
+            } else {
+                return transitionBundleTo(bndId, BundleState::INSTALLED);
+            }
+        }
+
+        bool uninstallBundle(long bndId) override {
+            bool uninstalled = false;
+            std::shared_ptr<celix::BundleController> removed{nullptr};
+            {
+                std::lock_guard<std::mutex> lck{bundles.mutex};
+                auto it = bundles.entries.find(bndId);
+                if (it != bundles.entries.end()) {
+                    removed = std::move(it->second);
+                    bundles.entries.erase(it);
+
+                }
+            }
+            if (removed) {
+                bool stopped = removed->transitionTo(BundleState::INSTALLED);
+                if (stopped) {
+                    //TODO check and wait till bundle is not used anymore. is this needed (shared_ptr) or just let access
+                    //to filesystem fail ...
+                } else {
+                    //add bundle again -> not uninstalled
+                    std::lock_guard<std::mutex> lck{bundles.mutex};
+                    bundles.entries[bndId] = std::move(removed);
+                }
+            }
+            return uninstalled;
+        }
+
+        bool transitionBundleTo(long bndId, BundleState desired) {
+            bool successful = false;
+            std::shared_ptr<celix::BundleController> match{nullptr};
+            {
+                std::lock_guard<std::mutex> lck{bundles.mutex};

Review comment:
       What if this match is removed in `uninstallBundle()`? I think the lock is not held long enough.

##########
File path: misc/experimental/cxx_framework/examples/Introduction/src/ComponentExample.cc
##########
@@ -0,0 +1,65 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <iostream>
+#include <chrono>
+#include "celix/Api.h"
+#include "celix/IShellCommand.h"
+#include "examples/IHelloWorld.h"
+
+namespace {
+    class ComponentExample : public celix::IShellCommand {
+    public:
+        void setHelloWorld(const std::shared_ptr<examples::IHelloWorld>& svc) {
+            std::lock_guard<std::mutex> lck{mutex};
+            helloWorld = svc;
+        }
+        void executeCommand(const std::string&, const std::vector<std::string>&, std::ostream &out, std::ostream &/*err*/) override {
+            std::lock_guard<std::mutex> lck{mutex};
+            if (helloWorld) {
+                out << helloWorld->sayHello() << std::endl;
+            } else {
+                out << "IHelloWorld service not available" << std::endl;
+            }
+        }
+    private:
+        std::mutex mutex{};
+        std::shared_ptr<examples::IHelloWorld> helloWorld{nullptr};
+    };
+
+    class ComponentExampleActivator : public celix::IBundleActivator {
+    public:
+        explicit ComponentExampleActivator(const std::shared_ptr<celix::BundleContext>& ctx) :

Review comment:
       Why a const ref to a shared_ptr? Why not a const ref to BundleContext?

##########
File path: misc/experimental/cxx_framework/libs/Framework/include/celix/BundleContext.h
##########
@@ -0,0 +1,156 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#pragma once
+
+
+#include "celix/Framework.h"
+#include "celix/IBundle.h"
+#include "celix/ServiceTrackerBuilder.h"
+#include "celix/ServiceRegistrationBuilder.h"
+#include "celix/UseServicesBuilder.h"
+#include "celix/ComponentManager.h"
+
+namespace celix {
+
+    class BundleContext {
+    public:
+        BundleContext() = default;
+        virtual ~BundleContext() = default;
+
+        BundleContext(celix::BundleContext &&) = delete;
+        BundleContext(const celix::BundleContext &) = delete;
+        BundleContext& operator=(celix::BundleContext &&) = delete;
+        BundleContext& operator=(const celix::BundleContext &) = delete;
+
+        template<typename I>
+        celix::ServiceRegistration registerService(std::shared_ptr<I> svc, celix::Properties props = {});
+
+        template<typename I>
+        celix::ServiceRegistration registerServiceFactory(std::shared_ptr<celix::IServiceFactory<I>> factory, celix::Properties props = {});
+
+        template<typename F>
+        celix::ServiceRegistration registerFunctionService(std::string_view functionName, F&& function, celix::Properties props = {});
+
+        template<typename I>
+        celix::ServiceRegistrationBuilder<I> buildServiceRegistration();
+
+        template<typename F>
+        celix::FunctionServiceRegistrationBuilder<F> buildFunctionServiceRegistration(std::string_view functionName);
+
+        template<typename I>
+        celix::ServiceTrackerBuilder<I> buildServiceTracker();
+
+//        template<typename F>
+//        celix::FunctionServiceTrackerBuilder<F> buildFunctionServiceTracker();
+
+        template<typename I>
+        celix::UseServicesBuilder<I> buildUseService();
+
+        template<typename F>
+        celix::UseFunctionServiceBuilder<F> buildUseFunctionService(std::string_view functionName);
+
+        template<typename T>
+        celix::ComponentManager<T> createComponentManager(std::shared_ptr<T> cmpInstance); //TODO factory / lazy component variant
+
+        template< typename I>
+        long findService(const celix::Filter& filter = celix::Filter{});
+
+        template<typename F>
+        long findFunctionService(const std::string &functionName, const celix::Filter& filter = celix::Filter{});
+        //TODO reg svc factories
+
+        virtual bool useBundle(long bndId, std::function<void(const celix::IBundle &bnd)> use) const = 0;
+
+        virtual int useBundles(std::function<void(const celix::IBundle &bnd)> use, bool includeFrameworkBundle = true) const = 0;
+
+        virtual bool stopBundle(long bndId) = 0;
+
+        virtual bool startBundle(long bndId) = 0;
+
+        //TODO install / uninstall bundles, use & track bundles
+
+        virtual std::shared_ptr<celix::IBundle> bundle() const = 0;

Review comment:
       Returning a shared_ptr suggests that it is valid for the user to store (increase refcounter) a bundle for later use. Is this an actual use case? If it is not really necessary, it might suffice to simply return a ref to the bundle. This also might avoid messy order of destruction problems.

##########
File path: misc/experimental/cxx_framework/libs/Framework/src/Bundle.cc
##########
@@ -0,0 +1,95 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <string>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "Bundle.h"
+
+celix::Bundle::Bundle(long _bndId, const std::string &cacheDir, celix::Framework *_fw, celix::Properties _manifest) :
+        bndId{_bndId}, fw{_fw}, bndManifest{std::move(_manifest)},
+        bundleCache{cacheDir + "/bundle" + std::to_string(_bndId)},
+        bndState{BundleState::INSTALLED} {
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_SYMBOLIC_NAME));
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_NAME));
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_GROUP));
+    assert(bndManifest.has(celix::MANIFEST_BUNDLE_VERSION));
+    bndState.store(BundleState::INSTALLED, std::memory_order_release);

Review comment:
       Assignment to bndState should also work, or `store()` without the 2nd argument.

##########
File path: misc/experimental/cxx_framework/libs/Framework/gtest/src/Framework_tests.cc
##########
@@ -0,0 +1,120 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#include <atomic>
+
+#include "gtest/gtest.h"
+
+#include "celix/Framework.h"
+
+class FrameworkTest : public ::testing::Test {
+public:
+    FrameworkTest() = default;
+    ~FrameworkTest() override = default;
+
+    celix::Framework& framework() { return *fw; }
+private:
+    std::shared_ptr<celix::Framework> fw{celix::Framework::create()};

Review comment:
       Why a shared_ptr? Would a plain 'value' suffice? e.g. `celix::Framework fw{celix::Framework::create()};`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org