| Summary: | Resolver hook not driven for State resolution operations | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Glyn Normington <glyn.normington> | ||||
| Component: | Framework | Assignee: | Thomas Watson <tjwatson> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | dmitry, hargrave, tjwatson | ||||
| Version: | 3.7 | ||||||
| Target Milestone: | 3.7 M5 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 330776, 332923 | ||||||
| Attachments: |
|
||||||
|
Description
Glyn Normington
I think the most simple solution would be to allow a ResolverHookFactory to be somehow passed when you create the snapshot State. Your ResolverHookFactory could then delegate to the hooks you need for isolation. Glyn, there is an internal method org.eclipse.osgi.internal.resolver.StateImpl.setResolverHookFactory(ResolverHookFactory) that you could try to use in the mean time to see if this idea works. I forgot to mention the class org.eclipse.osgi.framework.internal.core.CoreResolverHookFactory An instance of this class is used by the framework for the framework's State. It mainly does two things 1) creates shrinkable collections to prevent hooks from doing additions 2) delegates to the service registry to find the available hooks to call. It of coarse uses logs of registry internals to do this so that service hooks cannot hide registry hooks from the framework itself. Thanks Tom. I can certainly try setResolverHookFactory, but I thought from our earlier discussion that it would be tricky to pass BundleRevision and Capability instances when running a State disconnected from the framework. What's your thinking on that? The Descriptions from the State API (BundleDescription, ExportPackageDescrption etc) have a getCapability() method that the resolver implementation calls for the Capability objects and BundleDescription implements BundleRevision. The only thing that will not work (it will return null) is if you call BundleRevision.getBundle() on such a state that is not associated with the actual framework. That is my theory anyway. I would like you to give it a try and see if you hit any road blocks. Will do. Thanks. setResolverHookFactory seems to do the trick. :-)
The messiest part was getting hold of the bundle id of a bundle revision as in the following code:
private boolean isMember(BundleRevision bundleRevision) {
Bundle bundle = bundleRevision.getBundle();
if (bundle != null) {
return this.regionMembership.contains(bundle);
}
if (bundleRevision instanceof ResolverBundle) {
ResolverBundle resolverBundle = (ResolverBundle)bundleRevision;
return this.regionMembership.contains(resolverBundle.getBundleDescription().getBundleId());
}
Assert.isTrue(false, "Cannot determine region membership of BundleRevision '%s'", bundleRevision);
// Assume that bundle revisions with no bundles belong to the user region.
return true;
}
Perhaps getBundleId could be added to the BundleRevision interface as part of the fix to this bug.
The full set of code changes for the experiment is in this commit:
http://git.eclipse.org/c/virgo/org.eclipse.virgo.kernel.git/commit/?h=bug330776-framework-hooks&id=b0b9515da0abc922d60634ae5f56866fbe6978bd
BTW I didn't try using CoreResolverHookFactory since it didn't seem to add much value to the experiment and I couldn't easily get hold of the service registry constructor parameter.
(In reply to comment #6) > > Perhaps getBundleId could be added to the BundleRevision interface as part of > the fix to this bug. hmmm, that is OSGi API, unfortunately I'm not sure there is justification. The Bundle from the BundleReference should be used to get the id (but you don't have one). I doubt we can argue to add getBundleID. It would be the same as asking for BundleRevision to have a getBundleLocation() method. Where does it end for copying methods from Bundle to BundleRevision? The only reason getSymbolicName and getVersion and getTypes exist on the BundleRevision is because these could actually be different than the current revision used by the Bundle object. bug330776-framework-hooks&id=b0b9515da0abc922d60634ae5f56866fbe6978bd > > BTW I didn't try using CoreResolverHookFactory since it didn't seem to add much > value to the experiment and I couldn't easily get hold of the service registry > constructor parameter. Sorry to have mislead you there. I trying to be helpful in pointing you to some code that delegates to a set of hooks. But as you found out it has its hands in too many internals to be of much help. (In reply to comment #7) > hmmm, that is OSGi API, unfortunately I'm not sure there is justification. The > Bundle from the BundleReference should be used to get the id (but you don't > have one). I doubt we can argue to add getBundleID.... Agreed. > > > > BTW I didn't try using CoreResolverHookFactory since it didn't seem to add much > > value to the experiment and I couldn't easily get hold of the service registry > > constructor parameter. > > Sorry to have mislead you there. I trying to be helpful in pointing you to > some code that delegates to a set of hooks. But as you found out it has its > hands in too many internals to be of much help. No problem. (In reply to comment #7) > (In reply to comment #6) > > > > Perhaps getBundleId could be added to the BundleRevision interface as part of > > the fix to this bug. > > hmmm, that is OSGi API, unfortunately I'm not sure there is justification. The > Bundle from the BundleReference should be used to get the id (but you don't > have one). In the o.o.f.wiring package, BundleRevision.getBundle() can never return null. The o.o.f.wiring package is for an after-the-fact examination of the work product of the resolver. That is, the resolved bundles. An installed but not yet resolved bundle would not have a BundleWiring, but would have a BundleRevision which points back to the Bundle. I imagine the issue here relates to some internal state within the framework resolver where a BundleRevision is yet to have a Bundle object. But it does seem odd that a BundleRevision might not have a Bundle object? (In reply to comment #9) > (In reply to comment #7) > > (In reply to comment #6) > > > > > > Perhaps getBundleId could be added to the BundleRevision interface as part of > > > the fix to this bug. > > > > hmmm, that is OSGi API, unfortunately I'm not sure there is justification. The > > Bundle from the BundleReference should be used to get the id (but you don't > > have one). > > In the o.o.f.wiring package, BundleRevision.getBundle() can never return null. > The o.o.f.wiring package is for an after-the-fact examination of the work > product of the resolver. That is, the resolved bundles. An installed but not > yet resolved bundle would not have a BundleWiring, but would have a > BundleRevision which points back to the Bundle. > > I imagine the issue here relates to some internal state within the framework > resolver where a BundleRevision is yet to have a Bundle object. But it does > seem odd that a BundleRevision might not have a Bundle object? BJ, this is a case where glyn is using the Equinox resolver APIs to create a State that is disconnected from the framework so it has no Bundle objects associated with it. Glyn is doing a sandbox resolution so the resolver does not have access to the real Bundle Objects. (In reply to comment #6) > setResolverHookFactory seems to do the trick. :-) > > The messiest part was getting hold of the bundle id of a bundle revision as in > the following code: > > private boolean isMember(BundleRevision bundleRevision) { > Bundle bundle = bundleRevision.getBundle(); > if (bundle != null) { > return this.regionMembership.contains(bundle); > } > if (bundleRevision instanceof ResolverBundle) { > ResolverBundle resolverBundle = (ResolverBundle)bundleRevision; > return > this.regionMembership.contains(resolverBundle.getBundleDescription().getBundleId()); > } > Assert.isTrue(false, "Cannot determine region membership of > BundleRevision '%s'", bundleRevision); > // Assume that bundle revisions with no bundles belong to the user > region. > return true; > } > Glyn, I just released some changes to HEAD to make ALL BundleRevision objects passed to the hooks to be the actual BundleDescription objects since a BundleDescription implements BundleRevision. So you hack should be much better by simply trying to cast it to BundleDescription if getBundle returns null. Great. We'll pick this up in due course. I raised the closely related bug 333071 separately for clarity. Glyn, is there any update on the experiment? It seems rather successful. I am wondering if we should consider making State.setResolverHookFactory(ResolverHookFactory) API? The implementation should probably fail with an IllegalStateException if the hook has already been set to a non-null value. Otherwise we run the risk of letting someone changing the factory used by the framework resolver. (In reply to comment #14) > Glyn, is there any update on the experiment? It seems rather successful. I am > wondering if we should consider making > State.setResolverHookFactory(ResolverHookFactory) API? I agree it's a good approach and that method could be made an API. > > The implementation should probably fail with an IllegalStateException if the > hook has already been set to a non-null value. Otherwise we run the risk of > letting someone changing the factory used by the framework resolver. Agreed. Created attachment 186100 [details]
New API for setResolverHookFactory
Here is a patch for the new API. It will throw an IllegalStateException if the factory is already set. This behavior surfaced a bug in the framework for restarting. The BaseStorage was not correctly nulling out some objects. This left the framework in a stale state for restarts.
Patch released. |