eclipse-archived / concierge

Eclipse Concierge™ project
https://www.eclipse.org/concierge/
Eclipse Public License 1.0
33 stars 22 forks source link

removeBundleListener() does not work on framework bundle #9

Closed JochenHiller closed 7 years ago

JochenHiller commented 8 years ago

Migrated from Eclipse Bugzilla #493251

Simon Kaufmann 2016-05-09 10:58:54 EDT

When adding a BundleListener to the "framework bundle", this works nicely:

concierge.getBundleContext().addBundleListener(this);

However, removing it again does not work:

concierge.getBundleContext().removeBundleListener(this);

The reason clearly is the following in Concierge.BundleContextImpl.removeBundleListener():

if (bundle == Concierge.this) {
    return;
}

Maybe it's in there for a good reason, but I'm not sure if this asymmetry actually is intended.

JochenHiller commented 7 years ago

Thanks @sjka for reporting that. I have a test case which can easily reproduce that. @rellermeyer Can you say something about this code in Concierge.removeBundleListener() ?

if (bundle == Concierge.this) {
    return;
}

Was that introduced by intention?

JochenHiller commented 7 years ago

@rellermeyer I added a test case for that in a branch in my repo. See https://github.com/JochenHiller/concierge/commit/9e4e74a1b6de02cf58225a8190b2be1764389545 Simply copy the file to your workspace, and remove the "Ignore" annotation

JochenHiller commented 7 years ago

@rellermeyer For testing purposes I commented out that code. The test case and the complete test suite does work now without issues. Do you expect any big implications? I will try to run TCK before we will make that kind of change.

JochenHiller commented 7 years ago

Added a PR with the fix. When merging we should squash the commits into a single one.