capicxx-someip-runtime 3.2.4

- Unlock event processing - Fix build on Android - Call registerAvailabilityHandler before requestService - Fix deadlock issue when unregistering services - Added github workflow to build in Ubuntu - Fix: install boost - Build capicxx-someip-runtime on Windows - Fix Copyright & github link in README - Fix MethodStubDispatcher template - (dev) Warn about multiple subscriptions - Improve the CommonAPI-SomeIP Mainloop logs - Fix to check for the entry containing a valid Message obj to avoid nullptr - Fix Windows build
COVESA · Oct 8, 2024 · 86dfd69 · 86dfd69
1 parent 39f8f06
commit 86dfd69
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 42 deletions.
diff --git a/Android.bp b/Android.bp
@@ -6,13 +6,13 @@ cc_library_shared {
         "-D_GLIBCXX_USE_NANOSLEEP",
         "-DBOOST_LOG_DYN_LINK",
         "-pthread",
-	"-Wno-unused-private-field"
+        "-Wno-unused-private-field",
+        "-Wno-error=deprecated-declarations"
     ],
 
 
     local_include_dirs: [
         "include",
-        "internal",
     ],
 
     shared_libs: [

diff --git a/Android.mk b/Android.mk
@@ -34,8 +34,7 @@ LOCAL_SRC_FILES += \
     src/CommonAPI/SomeIP/Watch.cpp \
 
 LOCAL_C_INCLUDES := \
-    $(LOCAL_PATH)/include \
-    $(LOCAL_PATH)/internal
+    $(LOCAL_PATH)/include 
 
 LOCAL_SHARED_LIBRARIES := \
     libboost_log \

diff --git a/CHANGES b/CHANGES
@@ -1,6 +1,24 @@
 Changes
 =======
 
+v3.2.4
+- Unlock event processing
+- Fix build on Android
+- Call registerAvailabilityHandler before requestService
+- Fix deadlock issue when unregistering services
+- Added github workflow to build in Ubuntu
+- Fix: install boost
+- Build capicxx-someip-runtime on Windows
+
+v3.2.3-r9
+- Fix Copyright & github link in README
+- Fix MethodStubDispatcher template
+- (dev) Warn about multiple subscriptions
+- Improve the CommonAPI-SomeIP Mainloop logs
+- Fix to check for the entry containing a valid Message obj to avoid nullptr
+- Fix Windows build
+- Call registerAvailabilityHandler before requestService
+
 v3.2.3-r8
 - Changed level logs from warning to info when address_aliasing is activated
 - Fix incorrect timings in logs

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -11,7 +11,7 @@ set (CMAKE_VERBOSE_MAKEFILE off)
 
 set (LIBCOMMONAPI_SOMEIP_MAJOR_VERSION 3)
 set (LIBCOMMONAPI_SOMEIP_MINOR_VERSION 2)
-set (LIBCOMMONAPI_SOMEIP_PATCH_VERSION 3)
+set (LIBCOMMONAPI_SOMEIP_PATCH_VERSION 4)
 
 message(STATUS "Project name: ${PROJECT_NAME}")
 
@@ -66,7 +66,7 @@ include_directories( ${Boost_INCLUDE_DIR} )
 set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17 -D_CRT_SECURE_NO_WARNINGS /wd4503")
 link_directories(${Boost_LIBRARY_DIR})
 else()
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall -Wextra -Wformat -Wformat-security -Wconversion -fexceptions -fstrict-aliasing -fstack-protector -fasynchronous-unwind-tables -fno-omit-frame-pointer -DCOMMONAPI_INTERNAL_COMPILATION -D_GLIBCXX_USE_NANOSLEEP -DBOOST_LOG_DYN_LINK -pthread -fvisibility=hidden")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -Wall -Wextra -Wformat -Wformat-security -Wconversion -fexceptions -fstrict-aliasing -fstack-protector -fasynchronous-unwind-tables -fno-omit-frame-pointer -DCOMMONAPI_INTERNAL_COMPILATION -D_GLIBCXX_USE_NANOSLEEP -DBOOST_LOG_DYN_LINK -pthread -fvisibility=hidden")
 endif()
 
 SET(MAX_LOG_LEVEL "DEBUG" CACHE STRING "maximum log level")
@@ -214,3 +214,13 @@ endif()
 ##############################################################################
 # maintainer-clean
 add_custom_target(maintainer-clean COMMAND rm -rf *)
+
+##############################################################################
+# for debug
+if(MAINLOOP_DID_NOT_PROCESS_MESSAGE_MS_THRESHOLD)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DMAINLOOP_DID_NOT_PROCESS_MESSAGE_MS_THRESHOLD")
+endif()
+
+if(MAINLOOP_WATCH_ELEMENTS_SLOW_THRESHOLD)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DMAINLOOP_WATCH_ELEMENTS_SLOW_THRESHOLD")
+endif()
diff --git a/include/CommonAPI/SomeIP/Event.hpp b/include/CommonAPI/SomeIP/Event.hpp
@@ -70,7 +70,6 @@ class Event: public Events_  {
         }
 
         virtual void onEventMessage(const Message &_message) {
-            std::lock_guard<std::mutex> itsLock(notificationMutex_);
             if (auto ptr = proxy_.lock()) {
                 event_->handleEventMessage(_message, typename make_sequence<sizeof...(Arguments_)>::type());
             }
@@ -85,7 +84,6 @@ class Event: public Events_  {
     private :
         std::weak_ptr<ProxyBase> proxy_;
         Event<Events_, Arguments_ ...>* event_;
-        std::mutex notificationMutex_;
     };
 
     virtual void onFirstListenerAdded(const Listener&) {
@@ -103,6 +101,22 @@ class Event: public Events_  {
             std::lock_guard<std::mutex> itsLock(listeners_mutex_);
             listeners_.insert(_subscription);
         }
+        const std::size_t subscriptionCount = this->getSubscriptionCount();
+        if (subscriptionCount > 1) {
+            std::ostringstream identifiers;
+            identifiers << std::hex << std::setfill('0')
+                << std::setw(4) << serviceId_ << '.'
+                << std::setw(4) << instanceId_ << '.'
+                << std::setw(4) << eventId_;
+
+            COMMONAPI_WARNING(
+                "Detected multiple active subscriptions (",
+                subscriptionCount,
+                ") to the same event: [",
+                identifiers.str(),
+                "]. This is usually unintentional and may lead to poor performance."
+            );
+        }
 
         auto major = proxy_.getSomeIpAlias().getMajorVersion();
         proxy_.subscribe(serviceId_, instanceId_, eventgroupId_, eventId_,
@@ -121,10 +135,10 @@ class Event: public Events_  {
     template <size_t ... Indices_>
     inline void handleEventMessage(const Message &_message,
                                    index_sequence<Indices_...>) {
-
+        std::tuple<Arguments_...> itsArguments {arguments_};
         InputStream InputStream(_message, isLittleEndian_);
         if (SerializableArguments<Arguments_...>::deserialize(
-                InputStream, std::get<Indices_>(arguments_)...)) {
+                InputStream, std::get<Indices_>(itsArguments)...)) {
             if (_message.isInitialValue()) {
                 std::set<Subscription> subscribers;
                 {
@@ -133,21 +147,21 @@ class Event: public Events_  {
                     listeners_.clear();
                 }
                 for(auto const &subscription : subscribers) {
-                    if(!_message.isValidCRC()) {
+                    if (!_message.isValidCRC()) {
                         this->notifySpecificError(subscription, CommonAPI::CallStatus::INVALID_VALUE);
                     } else {
-                        this->notifySpecificListener(subscription, std::get<Indices_>(arguments_)...);
+                        this->notifySpecificListener(subscription, std::get<Indices_>(itsArguments)...);
                     }
                 }
             } else {
-                if(!_message.isValidCRC()) {
+                if (!_message.isValidCRC()) {
                     this->notifyErrorListeners(CommonAPI::CallStatus::INVALID_VALUE);
                 } else {
                     {
                         std::lock_guard<std::mutex> itsLock(listeners_mutex_);
                         listeners_.clear();
                     }
-                    this->notifyListeners(std::get<Indices_>(arguments_)...);
+                    this->notifyListeners(std::get<Indices_>(itsArguments)...);
                 }
             }
         } else {
@@ -166,9 +180,9 @@ class Event: public Events_  {
     const event_type_e eventType_;
     const reliability_type_e reliabilityType_;
     const bool isLittleEndian_;
-    std::tuple<Arguments_...> arguments_;
     std::mutex listeners_mutex_;
     std::set<Subscription> listeners_;
+    const std::tuple<Arguments_...> arguments_; // the deployments
 };
 
 } // namespace SomeIP

diff --git a/include/CommonAPI/SomeIP/OutputStream.hpp b/include/CommonAPI/SomeIP/OutputStream.hpp
@@ -568,7 +568,7 @@ class OutputStream: public CommonAPI::OutputStream<OutputStream> {
                 byte_t raw[sizeof(Type_)];
             } value;
             value.typed = _value;
-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__ANDROID__)
             if ((__BYTE_ORDER == __LITTLE_ENDIAN) != isLittleEndian_) {
 #else
             if (!isLittleEndian_) {

diff --git a/include/CommonAPI/SomeIP/StubAdapterHelper.hpp b/include/CommonAPI/SomeIP/StubAdapterHelper.hpp
@@ -338,7 +338,7 @@ class MethodStubDispatcher<StubClass_, In_<InArgs_...>, DeplIn_<DeplInArgs_...>>
 
         (_stub.get()->*stubFunctor_)(
             itsClientId,
-            std::move(std::get<InArgIndices_>(in))...
+            std::move(std::get<InArgIndices_>(in).getValue())...
         );
 
            return true;
@@ -405,7 +405,7 @@ class MethodWithReplyStubDispatcher<
         {
             std::lock_guard<std::mutex> lock(mutex_);
             auto itsMessage = pending_.find(_call);
-            if (itsMessage != pending_.end()) {
+            if (itsMessage != pending_.end() && itsMessage->second) {
                 Message itsReply = itsMessage->second.createResponseMessage();
                 pending_[_call] = itsReply;
             } else {
@@ -504,7 +504,7 @@ class MethodWithReplyStubDispatcher<
 
         std::lock_guard<std::mutex> lock(mutex_);
         auto reply = pending_.find(_call);
-        if (reply != pending_.end()) {
+        if (reply != pending_.end() && reply->second) {
             if (sizeof...(DeplOutArgs_) > 0) {
                 OutputStream output(reply->second, isLittleEndian_);
                 if (!SerializableArguments<CommonAPI::Deployable<OutArgs_, DeplOutArgs_>...>::serialize(
@@ -600,7 +600,7 @@ class MethodWithReplyStubDispatcher<
         {
             std::lock_guard<std::mutex> lock(this->mutex_);
             auto itsMessage = this->pending_.find(_call);
-            if (itsMessage != this->pending_.end()) {
+            if (itsMessage != this->pending_.end() && itsMessage->second) {
                 // TODO create error response message
                 Message itsReply = itsMessage->second.createResponseMessage();
                 this->pending_[_call] = itsReply;
@@ -689,7 +689,7 @@ class MethodWithReplyStubDispatcher<
 
         std::lock_guard<std::mutex> lock(this->mutex_);
         auto itsReply = this->pending_.find(_call);
-        if (itsReply != this->pending_.end()) {
+        if (itsReply != this->pending_.end() && itsReply->second) {
             if (sizeof...(DeplOutArgs_) > 0) {
                 OutputStream output(itsReply->second, this->isLittleEndian_);
                 if (!SerializableArguments<CommonAPI::Deployable<OutArgs_, DeplOutArgs_>...>::serialize(

diff --git a/src/CommonAPI/SomeIP/Connection.cpp b/src/CommonAPI/SomeIP/Connection.cpp
@@ -101,8 +101,9 @@ void Connection::handleProxyReceive(const std::shared_ptr<vsomeip::message> &_me
 
         // We must not hold the lock when calling the handlers!
         for (auto handler : handlers) {
-            if(auto itsHandler = handler.second.lock())
+            if(auto itsHandler = handler.second.lock()) {
                 itsHandler->onEventMessage(Message(_message));
+            }
         }
 
         return;

diff --git a/src/CommonAPI/SomeIP/Factory.cpp b/src/CommonAPI/SomeIP/Factory.cpp
@@ -192,6 +192,13 @@ Factory::registerStub(
         }
 
         CommonAPI::Address address(_domain, itsInterface, _instance);
+
+        if (isRegisteredService(address.getAddress())) {
+            COMMONAPI_ERROR("Registering stub for \"", _domain, ":", _interface, ":", _instance,
+                            "\" failed (Already registered).");
+            return false;
+        }
+
         Address someipAddress;
         AddressTranslator::get()->translate(address, someipAddress);
 
@@ -238,6 +245,13 @@ Factory::registerStub(
         }
 
         CommonAPI::Address address(_domain, itsInterface, _instance);
+
+        if (isRegisteredService(address.getAddress())) {
+            COMMONAPI_ERROR("Registering stub for \"", _domain, ":", _interface, ":", _instance,
+                            "\" failed (Already registered).");
+            return false;
+        }
+
         Address someipAddress;
         if (AddressTranslator::get()->translate(address, someipAddress)) {
             std::shared_ptr<Connection> itsConnection = getConnection(_context);
@@ -291,13 +305,12 @@ Factory::unregisterStub(const std::string &_domain,
     COMMONAPI_INFO("Deregistering stub for \"", _domain, ":", _interface, ":",
             _instance, "\"");
 
-    servicesMutex_.lock();
+    std::unique_lock<std::mutex> itsLock(servicesMutex_);
     CommonAPI::Address address(_domain, _interface, _instance);
 
     const auto &adapterResult = services_.find(address.getAddress());
 
     if (adapterResult == services_.end()) {
-        servicesMutex_.unlock();
         COMMONAPI_WARNING("Deregistering stub for \"", _domain, ":", _interface,
                 ":", _instance, "\" failed (Not registered or already unregistered).");
         return false;
@@ -311,13 +324,12 @@ Factory::unregisterStub(const std::string &_domain,
     stubManager->unregisterStubAdapter(adapter);
 
     if(!services_.erase(address.getAddress())) {
-        servicesMutex_.unlock();
         COMMONAPI_INFO("Deregistering stub for \"", _domain, ":", _interface,
                 ":", _instance, "\" failed (Removal failed).");
         return false;
     }
 
-    servicesMutex_.unlock();
+    itsLock.unlock();
 
     decrementConnection(connection);
 
@@ -352,6 +364,7 @@ Factory::createStubAdapter(
 
 bool
 Factory::isRegisteredService(const std::string &_address) {
+    std::lock_guard<std::mutex> itsLock(servicesMutex_);
     auto serviceIterator = services_.find(_address);
     if (serviceIterator != services_.end()) {
         return true;

diff --git a/src/CommonAPI/SomeIP/InstanceAvailabilityStatusChangedEvent.cpp b/src/CommonAPI/SomeIP/InstanceAvailabilityStatusChangedEvent.cpp
@@ -19,17 +19,9 @@ InstanceAvailabilityStatusChangedEvent::InstanceAvailabilityStatusChangedEvent(
                 observedInterfaceName_(_interfaceName),
                 observedInterfaceServiceId_ (_serviceId),
                 availabilityHandlerId_(0) {
-    Address wildcardAddress(observedInterfaceServiceId_, vsomeip::ANY_INSTANCE, vsomeip::ANY_MAJOR, vsomeip::ANY_MINOR);
-    proxy_.getConnection()->requestService(wildcardAddress);
 }
 
 InstanceAvailabilityStatusChangedEvent::~InstanceAvailabilityStatusChangedEvent() {
-    Address wildcardAddress(observedInterfaceServiceId_, vsomeip::ANY_INSTANCE,  vsomeip::ANY_MAJOR, vsomeip::ANY_MINOR);
-
-    proxy_.getConnection()->unregisterAvailabilityHandler(
-            wildcardAddress, availabilityHandlerId_);
-
-    proxy_.getConnection()->releaseService(wildcardAddress);
 }
 
 void
@@ -98,13 +90,19 @@ InstanceAvailabilityStatusChangedEvent::onFirstListenerAdded(
     std::weak_ptr<Proxy> itsProxy = proxy_.shared_from_this();
     availabilityHandlerId_ = proxy_.getConnection()->registerAvailabilityHandler(
                                 wildcardAddress, onServiceInstanceStatus, itsProxy, this);
+
+    proxy_.getConnection()->requestService(wildcardAddress);
 }
 
 void
 InstanceAvailabilityStatusChangedEvent::onLastListenerRemoved(
         const Listener &) {
-    // Destruktor of InstanceAvailabilityStatusChangedEvent will remove the availability handler
-    // As its needed anyways even if a user never subscribes on this event
+    Address wildcardAddress(observedInterfaceServiceId_, vsomeip::ANY_INSTANCE,  vsomeip::ANY_MAJOR, vsomeip::ANY_MINOR);
+
+    proxy_.getConnection()->unregisterAvailabilityHandler(
+            wildcardAddress, availabilityHandlerId_);
+
+    proxy_.getConnection()->releaseService(wildcardAddress);
 }
 
 bool

diff --git a/src/CommonAPI/SomeIP/Proxy.cpp b/src/CommonAPI/SomeIP/Proxy.cpp
@@ -289,8 +289,6 @@ bool Proxy::init() {
     if (!connection)
         return false;
 
-    connection->requestService(alias_);
-
     std::weak_ptr<Proxy> itsProxy = shared_from_this();
     availabilityHandlerId_ = connection->registerAvailabilityHandler(
                                     alias_,
@@ -303,6 +301,9 @@ bool Proxy::init() {
                                               std::placeholders::_5),
                                     itsProxy,
                                     NULL);
+
+    connection->requestService(alias_);
+
     if (connection->isAvailable(alias_)) {
         std::lock_guard<std::mutex> itsLock(availabilityMutex_);
         availabilityStatus_ = AvailabilityStatus::AVAILABLE;