Merge pull request #657 from apache/hotfix/653-cxx_remote_services_te…

…st_crash_stopgap Workaround the lifecycle management issue of IImportRegistration.
apache · Sep 26, 2023 · dcd8157 · dcd8157
2 parents e7aee12 + de76411
commit dcd8157
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 10 deletions.
diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml
@@ -21,7 +21,7 @@ jobs:
       fail-fast: false
       matrix:
         compiler: [ [gcc,g++], [clang,clang++] ]
-        type: [ Debug ]
+        type: [ Debug, RelWithDebInfo ]
     timeout-minutes: 120
     steps:
       - name: Checkout source code
@@ -100,6 +100,10 @@ jobs:
 
   linux-build-apt:
     runs-on: ubuntu-22.04
+    strategy:
+      fail-fast: false
+      matrix:
+        type: [ Debug, RelWithDebInfo ]
     timeout-minutes: 120
     steps:
     - name: Checkout source code
@@ -133,9 +137,9 @@ jobs:
       uses: actions/cache@v3
       with:
         path: ${{ env.CCACHE_DIR }}
-        key: ${{ runner.os }}-apt-test-ccache-gcc-Debug-${{ steps.ccache_cache_timestamp.outputs.timestamp }}
+        key: ${{ runner.os }}-apt-test-ccache-gcc-${{ matrix.type }}-${{ steps.ccache_cache_timestamp.outputs.timestamp }}
         restore-keys: |
-          ${{ runner.os }}-apt-test-ccache-gcc-Debug-
+          ${{ runner.os }}-apt-test-ccache-gcc-${{ matrix.type }}-
     - name: Build
       env:
         BUILD_OPTIONS: |
@@ -148,7 +152,7 @@ jobs:
           -DRSA_REMOTE_SERVICE_ADMIN_SHM_V2=ON
           -DSHELL_BONJOUR=ON
           -DENABLE_TESTING_ON_CI=ON
-          -DCMAKE_BUILD_TYPE=Debug
+          -DCMAKE_BUILD_TYPE=${{ matrix.type }}
           -DENABLE_CCACHE=ON
       run: |
         mkdir build install

diff --git a/bundles/cxx_remote_services/admin/src/RemoteServiceAdmin.cc b/bundles/cxx_remote_services/admin/src/RemoteServiceAdmin.cc
@@ -115,6 +115,7 @@ void celix::rsa::RemoteServiceAdmin::removeImportedServiceFactory(
     importServiceFactories.erase(targetServiceName);
 
     //TODO remove imported services from this factory ??needed
+    //FIXME yes, it will lead to crash if we uninstall the factory: https://github.com/apache/celix/issues/653
 }
 
 void celix::rsa::RemoteServiceAdmin::addExportedServiceFactory(

diff --git a/bundles/cxx_remote_services/integration/gtest/src/RemoteServicesIntegrationTestSuite.cc b/bundles/cxx_remote_services/integration/gtest/src/RemoteServicesIntegrationTestSuite.cc
@@ -57,8 +57,8 @@ class RemoteServicesIntegrationTestSuite : public ::testing::Test {
                 PS_PSA_BUNDLE_LOC,
                 PS_WIRE_BUNDLE_LOC,
                 RS_DISCOVERY_BUNDLE_LOC,
-                RS_RSA_BUNDLE_LOC,
-                RS_FACTORY_BUNDLE_LOC };
+                RS_FACTORY_BUNDLE_LOC ,
+                RS_RSA_BUNDLE_LOC};
         for (const auto& bndLoc : sharedBundles) {
             auto bndId = ctx->installBundle(bndLoc);
             EXPECT_GE(bndId, 0);

diff --git a/bundles/pubsub/integration/CMakeLists.txt b/bundles/pubsub/integration/CMakeLists.txt
@@ -329,9 +329,10 @@ if (PUBSUB_INTEGRATION)
     endif()
 
     if (BUILD_PUBSUB_PSA_WS)
+        find_package(civetweb REQUIRED)
         add_celix_container(pubsub_websocket_v2_tests
                 USE_CONFIG
-                LAUNCHER_SRC ${CMAKE_CURRENT_LIST_DIR}/gtest/PubSubIntegrationTestSuite.cc
+                LAUNCHER_SRC ${CMAKE_CURRENT_LIST_DIR}/gtest/PubSubWebsocketTestMain.cc
                 DIR ${CMAKE_CURRENT_BINARY_DIR}
                 PROPERTIES
                 LOGHELPER_STDOUT_FALLBACK_INCLUDE_DEBUG=true
@@ -346,7 +347,7 @@ if (PUBSUB_INTEGRATION)
                 pubsub_tst
                 pubsub_serializer
                 )
-        target_link_libraries(pubsub_websocket_v2_tests PRIVATE Celix::pubsub_api GTest::gtest GTest::gtest_main)
+        target_link_libraries(pubsub_websocket_v2_tests PRIVATE Celix::pubsub_api GTest::gtest civetweb::civetweb)
         target_include_directories(pubsub_websocket_v2_tests SYSTEM PRIVATE gtest)
         add_test(NAME pubsub_websocket_v2_tests COMMAND pubsub_websocket_v2_tests WORKING_DIRECTORY $<TARGET_PROPERTY:pubsub_websocket_v2_tests,CONTAINER_LOC>)
         setup_target_for_coverage(pubsub_websocket_v2_tests SCAN_DIR ..)

diff --git a/bundles/pubsub/integration/gtest/PubSubWebsocketTestMain.cc b/bundles/pubsub/integration/gtest/PubSubWebsocketTestMain.cc
@@ -0,0 +1,28 @@
+/*
+ * 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 <civetweb.h>
+#include "PubSubIntegrationTestSuite.cc"
+
+int main(int argc, char **argv) {
+    mg_init_library(MG_FEATURES_ALL);
+    ::testing::InitGoogleTest(&argc, argv);
+    int rc = RUN_ALL_TESTS();
+    mg_exit_library();
+    return rc;
+}
diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt b/bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt
@@ -15,6 +15,9 @@
 # specific language governing permissions and limitations
 # under the License.
 
+find_package(libffi REQUIRED)
+find_package(LibXml2 REQUIRED)
+
 add_celix_bundle(rsa_dfi_tst_bundle
     VERSION 0.0.1
     SOURCES
@@ -42,13 +45,16 @@ add_executable(test_rsa_dfi
 target_include_directories(test_rsa_dfi PRIVATE src)
 celix_deprecated_utils_headers(test_rsa_dfi)
 celix_deprecated_framework_headers(test_rsa_dfi)
+
 target_link_libraries(test_rsa_dfi PRIVATE
         civetweb::civetweb
         CURL::libcurl
         Celix::framework
         Celix::rsa_common
         calculator_api
         GTest::gtest
+        libffi::libffi
+        ${LIBXML2_LIBRARIES} # work around memory leak reported by ASAN
 )
 
 get_property(rsa_bundle_file TARGET rsa_dfi PROPERTY BUNDLE_FILE)

diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/main.cc b/bundles/remote_services/remote_service_admin_dfi/gtest/src/main.cc
@@ -18,12 +18,15 @@
  */
 
 #include <curl/curl.h>
+#include <civetweb.h>
 #include <gtest/gtest.h>
 
 int main(int argc, char **argv) {
     curl_global_init(CURL_GLOBAL_ALL);
+    mg_init_library(MG_FEATURES_ALL);
     ::testing::InitGoogleTest(&argc, argv);
     int rc = RUN_ALL_TESTS();
+    mg_exit_library();
     curl_global_cleanup();
     return rc;
 }
diff --git a/bundles/remote_services/topology_manager/tms_tst/CMakeLists.txt b/bundles/remote_services/topology_manager/tms_tst/CMakeLists.txt
@@ -16,6 +16,8 @@
 # under the License.
 
 find_package(CURL REQUIRED)
+find_package(civetweb REQUIRED)
+find_package(libffi REQUIRED)
 
 include_directories(
     ${PROJECT_SOURCE_DIR}/framework/public/include
@@ -44,6 +46,8 @@ target_link_libraries(test_tm_scoped PRIVATE
         calculator_api
         Celix::rsa_common
         CURL::libcurl
+        civetweb::civetweb
+        libffi::libffi
 )
 
 add_celix_bundle_dependencies(test_tm_scoped Celix::rsa_dfi Celix::rsa_topology_manager calculator topology_manager_disc_mock_bundle topology_manager_test_bundle)

diff --git a/bundles/remote_services/topology_manager/tms_tst/main.cc b/bundles/remote_services/topology_manager/tms_tst/main.cc
@@ -18,12 +18,15 @@
  */
 
 #include <curl/curl.h>
+#include <civetweb.h>
 #include <gtest/gtest.h>
 
 int main(int argc, char **argv) {
     curl_global_init(CURL_GLOBAL_ALL);
+    mg_init_library(MG_FEATURES_ALL);
     ::testing::InitGoogleTest(&argc, argv);
     int rc = RUN_ALL_TESTS();
+    mg_exit_library();
     curl_global_cleanup();
     return rc;
 }
diff --git a/conanfile.py b/conanfile.py
@@ -363,7 +363,8 @@ def configure(self):
             self.options['gtest'].shared = True
             if self.options.enable_address_sanitizer:
                 self.options["cpputest"].with_leak_detection = False
-        if self.options.build_rsa_discovery_common or self.options.build_shell_bonjour:
+        if (self.options.build_rsa_discovery_common or self.options.build_shell_bonjour or
+                (self.options.build_rsa_remote_service_admin_dfi and self.options.enable_testing)):
             self.options['libxml2'].shared = True
         if self.options.build_pubsub_psa_zmq:
             self.options['zeromq'].shared = True
@@ -389,7 +390,8 @@ def requirements(self):
             self.requires("libcurl/[>=7.64.1 <8.0.0]")
         if self.options.build_deployment_admin:
             self.requires("zlib/[>=1.2.8 <2.0.0]")
-        if self.options.build_rsa_discovery_common or self.options.build_shell_bonjour:
+        if (self.options.build_rsa_discovery_common or self.options.build_shell_bonjour or
+                (self.options.build_rsa_remote_service_admin_dfi and self.options.enable_testing)):
             self.requires("libxml2/[>=2.9.9 <3.0.0]")
         if self.options.build_cxx_remote_service_admin:
             self.requires("rapidjson/[>=1.1.0 <2.0.0]")