Skip to content

701 Remove deadlock in config notification (#1040)#1174

Merged
jeffdiclemente merged 2 commits intoCppMicroServices:c++14-compliantfrom
insi-eb:701_config_notification
Jul 29, 2025
Merged

701 Remove deadlock in config notification (#1040)#1174
jeffdiclemente merged 2 commits intoCppMicroServices:c++14-compliantfrom
insi-eb:701_config_notification

Conversation

@RobertLauferElektrobit
Copy link
Copy Markdown

  • added test to verify that update call from inside modified no longer causes deadlock
  • memory leaks... no idea why they are happening but 99% sure this is a false positive
  • fix false leak
  • respond to jeff comments
  • respond to conor 's comment

cherry-pick of commit 2e4eca4 (PR #1040)

see discussion #701

* added test to verify that update call from inside modified no longer causes deadlock

* memory leaks... no idea why they are happening but 99% sure this is a false positive

* fix false leak

* respond to jeff comments

* respond to conor
's comment

---------

Co-authored-by: Toby Cormack <tcormack@mathworks.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

{
threadpool->join();
}
catch (...)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]

                catch (...)
                ^

}
}
}
catch (...)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]

        catch (...)
        ^

}

void
AsyncWorkServiceThreadPool::post(std::packaged_task<void()>&& task)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: rvalue reference parameter 'task' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]

    AsyncWorkServiceThreadPool::post(std::packaged_task<void()>&& task)
                                                                  ^

*/
bool isBundleLoadedInThisProcess(std::string bundleName);

class AsyncWorkServiceThreadPool : public cppmicroservices::async::AsyncWorkService
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: class 'AsyncWorkServiceThreadPool' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

    class AsyncWorkServiceThreadPool : public cppmicroservices::async::AsyncWorkService
          ^

@@ -34,10 +34,7 @@
#include "Mocks.hpp"
#include "TestFixture.hpp"
#include "TestInterfaces/Interfaces.hpp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'TestInterfaces/Interfaces.hpp' file not found [clang-diagnostic-error]

#include "TestInterfaces/Interfaces.hpp"
         ^

ASSERT_FALSE(GetInstance<test::CAInterface>())
<< "Factory component should not be registered even with config matching exactly (with no ~)";

auto uniqueVal1 = 6;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 6 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

        auto uniqueVal1 = 6;
                          ^

namespace sample
{

ServiceComponentCA10::ServiceComponentCA10(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: properties, CA [cppcoreguidelines-pro-type-member-init]

compendium/test_bundles/TestBundleDSCA10/src/ServiceImpl.hpp:27:

-         std::shared_ptr<cppmicroservices::AnyMap> properties;
-         std::shared_ptr<cppmicroservices::service::cm::ConfigurationAdmin> CA;
+         std::shared_ptr<cppmicroservices::AnyMap> properties{};
+         std::shared_ptr<cppmicroservices::service::cm::ConfigurationAdmin> CA{};

@@ -0,0 +1,33 @@
#ifndef _SERVICE_IMPL_HPP_
#define _SERVICE_IMPL_HPP_
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: declaration uses identifier 'SERVICE_IMPL_HPP', which is a reserved identifier [bugprone-reserved-identifier]

Suggested change
#define _SERVICE_IMPL_HPP_
#define SERVICE_IMPL_HPP_

#ifndef _SERVICE_IMPL_HPP_
#define _SERVICE_IMPL_HPP_

#include "TestInterfaces/Interfaces.hpp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'TestInterfaces/Interfaces.hpp' file not found [clang-diagnostic-error]

#include "TestInterfaces/Interfaces.hpp"
         ^


namespace sample
{
class ServiceComponentCA10 : public test::CAInterface
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: class 'ServiceComponentCA10' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

    class ServiceComponentCA10 : public test::CAInterface
          ^

@jeffdiclemente jeffdiclemente merged commit 73bba2f into CppMicroServices:c++14-compliant Jul 29, 2025
0 of 51 checks passed
@RobertLauferElektrobit RobertLauferElektrobit deleted the 701_config_notification branch July 29, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants