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

Segmentation error after unloading module #2

Closed
magnet opened this Issue May 8, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@magnet

magnet commented May 8, 2012

Follow up of code on forum http://forum.cppmicroservices.org/viewtopic.php?id=6

I now try to unload libA and check that there are no service references available.

During the call to GetModuleContext()->GetServiceReference("foo"), the program crashes.

The output ends with:

crash?
get service ref foo for module MainModule = 0 refs
Segmentation error

This happens during the call to GetServiceReference(), and the debug trace shows there are now 0 refs (compared to 1 before unloading). The earlier code shows the module as unloaded.

 libA.Unload();

  cout << "After unload" << endl;

  for (auto it = modules.begin(); it != modules.end(); ++it) {
        cout << (*it)->GetName() << endl;
    }

    moduleA = ModuleRegistry::GetModule("DictionaryServiceModule");

    if (moduleA) {

        cout << moduleA->IsLoaded() << endl;
    } else {
        cout << "errrrrrrror" << endl;
    }

    refs = GetModuleContext()->GetServiceReferences("");
    for (auto it = refs.begin(); it != refs.end(); ++it) {
        cout << (*it).GetProperty(ServiceConstants::OBJECTCLASS()) << endl;
    }

  cout<< "crash?"<<endl;

    //![GetDictionaryService]
    dictionaryServiceRef =
            GetModuleContext()->GetServiceReference("foo");
    if (dictionaryServiceRef) {
        DictionaryService* dictionaryService = (DictionaryService*) GetModuleContext()->GetService(dictionaryServiceRef);
        if (dictionaryService) {
            std::cout << "Dictionary contains 'Tutorial': "
                    << dictionaryService->checkWord("Tutorial") << std::endl;
        }
    }


    return 0;
@saschazelzer

This comment has been minimized.

Show comment
Hide comment
@saschazelzer

saschazelzer May 8, 2012

Member

Hi,

I'm afraid that this is not a bug in CppMicroServices ;-)

Unloading libraries is dangerous. In your case, you are still referencing objects (ServiceReference instances) which themselves reference objects which were instantiated in the DictionaryService module. Unloading the module and than trying to use one of these service references (by using, I also mean the automatic destruction of them when reaching the end of their scope) is illegal.

To be able to play with unloading, you could nest the variables created before the libA.Unload() call in an additional scope such that they are destructed earlier. E.g.:

Module* moduleA = ModuleRegistry::GetModule("DictionaryService Module");
{
  ServiceReference dictionaryServiceRef =
      GetModuleContext()->GetServiceReference("foo");
  // do something with the reference
}
libA.Unload();
Member

saschazelzer commented May 8, 2012

Hi,

I'm afraid that this is not a bug in CppMicroServices ;-)

Unloading libraries is dangerous. In your case, you are still referencing objects (ServiceReference instances) which themselves reference objects which were instantiated in the DictionaryService module. Unloading the module and than trying to use one of these service references (by using, I also mean the automatic destruction of them when reaching the end of their scope) is illegal.

To be able to play with unloading, you could nest the variables created before the libA.Unload() call in an additional scope such that they are destructed earlier. E.g.:

Module* moduleA = ModuleRegistry::GetModule("DictionaryService Module");
{
  ServiceReference dictionaryServiceRef =
      GetModuleContext()->GetServiceReference("foo");
  // do something with the reference
}
libA.Unload();
@saschazelzer

This comment has been minimized.

Show comment
Hide comment
@saschazelzer

saschazelzer May 8, 2012

Member

As a side note: Also in OSGi, unloading a bundle does not necessarily remove the jar file completely from the running system (as a dlclose does with a shared library if the load count drops to zero). Classes from exported packages may still be referenced and the garbage collector "keeps" the packages around. There is no such safety net in C/C++.

Member

saschazelzer commented May 8, 2012

As a side note: Also in OSGi, unloading a bundle does not necessarily remove the jar file completely from the running system (as a dlclose does with a shared library if the load count drops to zero). Classes from exported packages may still be referenced and the garbage collector "keeps" the packages around. There is no such safety net in C/C++.

@magnet

This comment has been minimized.

Show comment
Hide comment
@magnet

magnet May 8, 2012

Thanks! It's obvious now, but as you said I'm used to Java where a service can be a stalled reference and the owning classloader is not collected until all references are freed. I wasn't expecting a segmentation violation from this, but I understand how it could happen. I got it working with by scoping the objects depending on the load as you said, I'll close the issue :).

magnet commented May 8, 2012

Thanks! It's obvious now, but as you said I'm used to Java where a service can be a stalled reference and the owning classloader is not collected until all references are freed. I wasn't expecting a segmentation violation from this, but I understand how it could happen. I got it working with by scoping the objects depending on the load as you said, I'll close the issue :).

@magnet magnet closed this May 8, 2012

saschazelzer pushed a commit that referenced this issue Sep 22, 2015

Remove static globals
This includes a few other changes required to be able to remove static globals:

- "Framework" and "Framework Factory" classes as a single entry point for clients to initialize CppMicroServices.
- A minimal lifecycle for bundles, consisting solely of "install", "start", "stop" and "uninstall".
- Bundles are installed first and then loaded into the process only on bundle activation.
- Transition to declaring the bundle name in the bundle's manifest file.
- Auto-loading needs to be either factored out into a service or modified to allow auto-installing of bundles.

Closes #2

Signed-off-by: The MathWorks, Inc. Roy.Lurie@mathworks.com

ksubramz added a commit to ksubramz/CppMicroServices that referenced this issue Mar 15, 2017

Support multiple listeners.
Change the container holding futures to array, to remove Run time check error #2
reported by VS2013. The issue is pending investigation.

jeffdiclemente added a commit that referenced this issue Apr 12, 2017

Support multiple listeners. (#179)
* Support multiple listeners.

Fix issues #83 and #95.

Change the BundleContext add listeners functions to return ListenerToken
objects.

Add API function RemoveListeners which can use these
ListenerToken objects to remove the listeners corresponding to the
tokens. Removing listeners using their tokens is the recommended
approach to clients going forward.

Introduce the move-only ListenerToken type. This is a wrapper to
ListenerTokenId, an uint64_t type, which is incremented every-time
a listener of any type is added to the BundleContext.

Deprecate existing API functions RemoveFrameworkListener,
RemoveBundleListener and RemoveServiceListener. These will be
removed in the next major release. These functions also emit
diagnostic log messages that they are deprecated.

Change framework code to use tokens in-place of the deprecated
RemoveServiceListener calls.

Change ServiceListenerEntry to contain ListenerTokenId.

Enable the previously commented out multiple framework listeners
test block in FrameworkListenerTest.

Add a new test file named MultipleListenersTest which exercises
this new functionality in a variety of ways.

Signed-off-by: The Mathworks Inc <Roy.Lurie@mathworks.com>

* Support multiple listeners.

Fix issues in Mac. Temporarily removed US_DEPRECATED because it resulted in
build failures.

* Support multiple listeners.

Fix based on Sascha's PR review.

Modify bundle and framework listeners to be an unordered_map with
ListenerTokenIds as keys. This improves the removal of these types of listeners
to O(1).

Deprecate addition of listeners functions which take member functions of
objects. Clients can use std::bind together with the other Add*Listener
functions to achieve the same result.

Modify RemoveListener call to take token by value. With this change, clients
have to explicitly move the tokens.

* Support multiple listeners.

Change the container holding futures to array, to remove Run time check error #2
reported by VS2013. The issue is pending investigation.

* Support multiple listeners.

Incorporate suggestions by Jeff and Abhinay after code review.

Clean up comments.
Introduce mechanisms in the test code to remove warnings due to deprecated
functions. De-deprecate Add.*Listener functions which take member
functions.
Reduce code duplication by cleaning up unneeded constructors.
Improve concurrency test of addition multiple listeners.

* Support multiple listeners.

Fix because of Mac build failures. Minor improvements.

* Support multiple listeners.

Fix GCC 4.9.2 build failures. Move the disabling of deprecation warning up to
CMake level and add means to disable the deprecation warning per source file.

Deprecate the Add.*Listener functions that take the member function pointers.
Modify the instances where service listeners are added to take the std::bind
objects instead.

* Support multiple listeners.

Remove a stray trailing macro.

* Support multiple listeners.

Fix a sporadic threading related crash by usage of Framework::WaitForStop().

Replace ordered_map::emplace use with [] because emplace isn't supported by
older GCC compilers.

* Support mutliple listeners.

Change based on Sascha's review.

* Support multiple listeners.

Add a test that adds and removes listeners in a true concurrent fashion.

Change based on Sascha and Jeff's comments.

* Fix Mac build failures.

* Support multiple listeners.

Added a workaround for a Visual Studio 2013 compiler bug.

* Support multiple listeners.

Factor out ListenerToken into a separate header and implementation file.

* Support multiple listeners.

Add missing file

* Support multiple changes.

Fix based on review

* Support multiple listeners.

Improve minor things - remove unnecessary constructor and fix API doc.

* Support multiple listeners.

Templatize testConcurrentAddRemove function to work with all 3 types of
listeners.

* Support multiple listeners.

Fix Clang bug in Mac.

* Support multiple listeners.

Fix to build on GCC 4.6

* Support multiple listeners.

Change based on Jeff's review.

* Support multiple listeners.

Change based on review (minor).

Signed-off-by: The Mathworks Inc Roy.Lurie@mathworks.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment