Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #840: removes manual reference counting #841

Merged
merged 38 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c2973b0
converted registration
tcormackMW Mar 16, 2023
11ef3c2
compiled, not passeD
tcormackMW Mar 16, 2023
e3cd77a
fixed weak_ptr error, now leaking mock objects
tcormackMW Mar 17, 2023
da8de67
fixed bug, need to decrement ref count
tcormackMW Mar 21, 2023
e8fc75d
updats from last week, test cases failing because of out of date expe…
tcormackMW Mar 28, 2023
7c0e263
all tests are passing with shared and weak pointers to serviceRegistr…
tcormackMW Apr 11, 2023
4f6356e
still passing tests, updated comments and cleaned up
tcormackMW Apr 11, 2023
73fa52a
updating to share dependents
tcormackMW Apr 12, 2023
efd22a5
new issue with dying service
tcormackMW Apr 13, 2023
6f74217
added coreInfo, maybe passing
tcormackMW Apr 17, 2023
c8355db
passing tests, removed manual ref counting from referenceBasePrivate,…
tcormackMW Apr 18, 2023
9a91e9e
updated comments
tcormackMW Apr 18, 2023
43357cc
updates before PR
tcormackMW Apr 18, 2023
60419ce
Removed manual reference counting
tcormackMW Apr 19, 2023
abf6b3f
Merge remote-tracking branch 'upstream/development' into sharedPtr
tcormackMW Apr 19, 2023
ebf86e4
Removed manual reference counting and merged with upstream (#840)
tcormackMW Apr 19, 2023
e0df823
ServiceRegistrationCoreInfo now default destructor
tcormackMW Apr 19, 2023
7d7bdd6
Updated based on Patty's comments #840
tcormackMW Apr 20, 2023
976bdd4
updated ServiceReferenceBase Constructors for clarity with shared_ptrs
tcormackMW Apr 21, 2023
75d4736
removed 'move' from serviceRegistry
tcormackMW Apr 24, 2023
4f2da93
attempt at solving mac issue
tcormackMW May 1, 2023
b24d794
updates for lock type and removing unneccessary functions from reference
tcormackMW May 1, 2023
230c749
lost lock
tcormackMW May 9, 2023
5f236b9
Merge branch 'CppMicroServices:development' into sharedPtr
tcormackMW May 9, 2023
7def5ac
changed to custom atomic load
tcormackMW May 9, 2023
93d25d9
LockSet addition
tcormackMW May 10, 2023
9d3133f
Merge branch 'CppMicroServices:development' into sharedPtr
tcormackMW May 10, 2023
36953b6
threading support in LockSet
tcormackMW May 10, 2023
4e5f432
LockSet not threaded
tcormackMW May 10, 2023
8444b5c
no names in func dec
tcormackMW May 10, 2023
5fc7802
Incoorporated Jeff's Comments
tcormackMW May 11, 2023
85a5d6d
Assignment operator didn't fail on my computer, did in github
tcormackMW May 11, 2023
40e4aee
Merge branch 'CppMicroServices:development' into sharedPtr
tcormackMW May 12, 2023
4463057
updated for Jeff's 5/16 comments
tcormackMW May 16, 2023
1db6243
clang update
tcormackMW May 16, 2023
8eb6855
mikes comments and fixes for multithreaded support
tcormackMW May 17, 2023
f957cd8
Merge branch 'CppMicroServices:development' into sharedPtr
tcormackMW May 17, 2023
c602217
remove ifdefs from BundleRegistry, abide by rule of (0,3,5), and add …
tcormackMW May 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ doc/src/examples/makefile/main
*.pyc
/build*
.vscode/
.venv/
.venv/
10 changes: 7 additions & 3 deletions framework/include/cppmicroservices/ServiceReferenceBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "cppmicroservices/Any.h"
#include <functional>
#include "cppmicroservices/detail/Threads.h"

#include <atomic>
#include <memory>
Expand Down Expand Up @@ -232,13 +233,16 @@ namespace cppmicroservices
*/
ServiceReferenceBase();

ServiceReferenceBase(ServiceRegistrationBasePrivate* reg);
ServiceReferenceBase(std::shared_ptr<ServiceRegistrationBasePrivate> reg);

void SetInterfaceId(std::string const& interfaceId);

// This class is not thread-safe, but we support thread-safe
// copying and assignment.
std::atomic<ServiceReferenceBasePrivate*> d;
// copying and assignment
// This was changed to a std::shared_ptr and is accessed through
// Load(), but when this repository uses c++20,
// this will be changed to std::atomic<shared_ptr>
cppmicroservices::detail::Atomic<std::shared_ptr<ServiceReferenceBasePrivate>> d;
jeffdiclemente marked this conversation as resolved.
Show resolved Hide resolved
};

/**
Expand Down
37 changes: 20 additions & 17 deletions framework/include/cppmicroservices/ServiceRegistrationBase.h
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
/*=============================================================================
/*=============================================================================

Library: CppMicroServices
Library: CppMicroServices

Copyright (c) The CppMicroServices developers. See the COPYRIGHT
file at the top-level directory of this distribution and at
https://github.com/CppMicroServices/CppMicroServices/COPYRIGHT .
Copyright (c) The CppMicroServices developers. See the COPYRIGHT
file at the top-level directory of this distribution and at
https://github.com/CppMicroServices/CppMicroServices/COPYRIGHT .

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

=============================================================================*/

Expand All @@ -30,6 +30,7 @@
namespace cppmicroservices
{

class RegistrationLocks;
class BundlePrivate;
jeffdiclemente marked this conversation as resolved.
Show resolved Hide resolved
class ServiceRegistrationBasePrivate;
class Properties;
Expand Down Expand Up @@ -196,11 +197,13 @@ namespace cppmicroservices
*/
ServiceRegistrationBase();

ServiceRegistrationBase(ServiceRegistrationBasePrivate* registrationPrivate);
ServiceRegistrationBase(std::shared_ptr<ServiceRegistrationBasePrivate> registrationPrivate);

ServiceRegistrationBase(BundlePrivate* bundle, InterfaceMapConstPtr const& service, Properties&& props);

ServiceRegistrationBasePrivate* d { nullptr };
std::shared_ptr<RegistrationLocks> LockRegistration() const;

jeffdiclemente marked this conversation as resolved.
Show resolved Hide resolved
std::shared_ptr<ServiceRegistrationBasePrivate> d;
};

/**
Expand All @@ -224,7 +227,7 @@ namespace cppmicroservices
*/

US_HASH_FUNCTION_BEGIN(cppmicroservices::ServiceRegistrationBase)
return std::hash<cppmicroservices::ServiceRegistrationBasePrivate*>()(arg.d);
return std::hash<std::shared_ptr<cppmicroservices::ServiceRegistrationBasePrivate>>()(arg.d);
US_HASH_FUNCTION_END

#endif // CPPMICROSERVICES_SERVICEREGISTRATIONBASE_H
4 changes: 4 additions & 0 deletions framework/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ set(_srcs
util/SharedLibrary.cpp
util/SharedLibraryException.cpp
util/Utils.cpp
util/RegistrationLocks.cpp

service/ListenerToken.cpp
service/ServiceException.cpp
Expand All @@ -34,6 +35,7 @@ set(_srcs
service/ServiceReferenceBasePrivate.cpp
service/ServiceRegistrationBase.cpp
service/ServiceRegistrationBasePrivate.cpp
service/ServiceRegistrationCoreInfo.cpp
service/ServiceRegistry.cpp

bundle/Bundle.cpp
Expand Down Expand Up @@ -67,13 +69,15 @@ set(_private_headers
util/Properties.h
util/PropsCheck.h
util/Utils.h
util/RegistrationLocks.h

service/ServiceHooks.h
service/ServiceListenerEntry.h
service/ServiceListenerHookPrivate.h
service/ServiceListeners.h
service/ServiceReferenceBasePrivate.h
service/ServiceRegistrationBasePrivate.h
service/ServiceRegistrationCoreInfo.h
service/ServiceRegistry.h

bundle/BundleArchive.h
Expand Down
7 changes: 3 additions & 4 deletions framework/src/bundle/BundleContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ namespace cppmicroservices
{
try
{
sref.d.load()->UngetService(b.lock(), true);
//sref.d.load()->UngetService(b.lock(), true);
sref.d.Load()->UngetService(b.lock(), true);
}
catch (...)
{
Expand Down Expand Up @@ -312,7 +311,7 @@ namespace cppmicroservices
auto b = GetAndCheckBundlePrivate(d);

std::shared_ptr<ServiceHolder<void>> h(
new ServiceHolder<void>(b, reference, reference.d.load()->GetService(b.get())));
new ServiceHolder<void>(b, reference, reference.d.Load()->GetService(b.get())));
return std::shared_ptr<void>(h, h->service.get());
}

Expand All @@ -333,7 +332,7 @@ namespace cppmicroservices
d->CheckValid();
auto b = GetAndCheckBundlePrivate(d);

auto serviceInterfaceMap = reference.d.load()->GetServiceInterfaceMap(b.get());
auto serviceInterfaceMap = reference.d.Load()->GetServiceInterfaceMap(b.get());
std::shared_ptr<ServiceHolder<InterfaceMap const>> h(
new ServiceHolder<InterfaceMap const>(b, reference, serviceInterfaceMap));
return InterfaceMapConstPtr(h, h->service.get());
Expand Down
6 changes: 3 additions & 3 deletions framework/src/bundle/BundleHooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ namespace cppmicroservices
for (auto srBaseIter = srl.rbegin(), srBaseEnd = srl.rend(); srBaseIter != srBaseEnd; ++srBaseIter)
{
ServiceReference<BundleFindHook> sr = srBaseIter->GetReference();
std::shared_ptr<BundleFindHook> fh
= std::static_pointer_cast<BundleFindHook>(sr.d.load()->GetService(GetPrivate(selfBundle).get()));
std::shared_ptr<BundleFindHook> fh = std::static_pointer_cast<BundleFindHook>(
sr.d.Load()->GetService(GetPrivate(selfBundle).get()));
if (fh)
{
try
Expand Down Expand Up @@ -139,7 +139,7 @@ namespace cppmicroservices
}

std::shared_ptr<BundleEventHook> eh = std::static_pointer_cast<BundleEventHook>(
sr.d.load()->GetService(GetPrivate(GetBundleContext().GetBundle()).get()));
sr.d.Load()->GetService(GetPrivate(GetBundleContext().GetBundle()).get()));
if (eh)
{
try
Expand Down
3 changes: 2 additions & 1 deletion framework/src/bundle/BundlePrivate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,8 @@
coreCtx->services.GetUsedByBundle(this, srs);
for (std::vector<ServiceRegistrationBase>::const_iterator i = srs.begin(); i != srs.end(); ++i)
{
i->GetReference(std::string()).d.load()->UngetService(this->shared_from_this(), false);
auto ref = i->GetReference(std::string());
ref.d.Load()->UngetService(this->shared_from_this(), false);

Check warning on line 827 in framework/src/bundle/BundlePrivate.cpp

View check run for this annotation

Codecov / codecov/patch

framework/src/bundle/BundlePrivate.cpp#L826-L827

Added lines #L826 - L827 were not covered by tests
}
}

Expand Down
6 changes: 3 additions & 3 deletions framework/src/service/ServiceHooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ namespace cppmicroservices
for (auto fhrIter = srl.rbegin(), fhrEnd = srl.rend(); fhrIter != fhrEnd; ++fhrIter)
{
ServiceReference<ServiceFindHook> sr = fhrIter->GetReference();
auto fh
= std::static_pointer_cast<ServiceFindHook>(sr.d.load()->GetService(GetPrivate(selfBundle).get()));
auto fh = std::static_pointer_cast<ServiceFindHook>(
sr.d.Load()->GetService(GetPrivate(selfBundle).get()));
if (fh)
{
try
Expand Down Expand Up @@ -177,7 +177,7 @@ namespace cppmicroservices
{
ServiceReference<ServiceEventListenerHook> sr = sriIter->GetReference();
auto elh = std::static_pointer_cast<ServiceEventListenerHook>(
sr.d.load()->GetService(GetPrivate(selfBundle).get()));
sr.d.Load()->GetService(GetPrivate(selfBundle).get()));
if (elh)
{
try
Expand Down
2 changes: 1 addition & 1 deletion framework/src/service/ServiceListeners.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ namespace cppmicroservices
// Get a copy of the service reference and keep it until we are
// done with its properties.
auto ref = evt.GetServiceReference();
auto props = ref.d.load()->GetProperties();
auto props = ref.d.Load()->GetProperties();

{
auto l = this->Lock();
Expand Down
8 changes: 4 additions & 4 deletions framework/src/service/ServiceObjects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ namespace cppmicroservices

if (isPrototypeScope)
{
result = m_reference.d.load()->GetPrototypeService(MakeBundleContext(m_context).GetBundle());
result = m_reference.d.Load()->GetPrototypeService(MakeBundleContext(m_context).GetBundle());
}
else
{
result = m_reference.d.load()->GetServiceInterfaceMap(
result = m_reference.d.Load()->GetServiceInterfaceMap(
GetPrivate(MakeBundleContext(m_context).GetBundle()).get());
}

Expand Down Expand Up @@ -115,11 +115,11 @@ namespace cppmicroservices

if (isPrototypeScope)
{
sref.d.load()->UngetPrototypeService(bundle, interfaceMap);
sref.d.Load()->UngetPrototypeService(bundle, interfaceMap);
}
else
{
sref.d.load()->UngetService(bundle, true);
sref.d.Load()->UngetService(bundle, true);
}
}
}
Expand Down
Loading
Loading