Bug 575576

Summary: BundleWiring and Wiring lists must be snapshot/mutable
Product: [Eclipse Project] Equinox Reporter: Thomas Watson <tjwatson>
Component: FrameworkAssignee: Thomas Watson <tjwatson>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: wellmann.hannes1
Version: 4.21Keywords: regression
Target Milestone: 4.21 RC1   
Hardware: PC   
OS: Mac OS X   
See Also: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/184329
https://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=d231f4aaf90c9fcee6069ced7e5cf836a0f4183c
https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/184541
https://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=f2d9c2ca7f83e21a5761c42025d0c41cb789675d
Whiteboard:

Description Thomas Watson CLA 2021-08-23 12:33:08 EDT 
The following methods on BundleWiring all state they return "A list containing a snapshot of the...":

org.osgi.framework.wiring.BundleWiring.getCapabilities(String)
org.osgi.framework.wiring.BundleWiring.getProvidedResourceWires(String)
org.osgi.framework.wiring.BundleWiring.getProvidedWires(String)
org.osgi.framework.wiring.BundleWiring.getRequiredResourceWires(String)
org.osgi.framework.wiring.BundleWiring.getRequiredWires(String)
org.osgi.framework.wiring.BundleWiring.getRequirements(String)
org.osgi.framework.wiring.BundleWiring.getResourceCapabilities(String)
org.osgi.framework.wiring.BundleWiring.getResourceRequirements(String)

Prior to the fix for bug 572605 these methods did return a new copy (snapshot) that was mutable by the caller.  Now they return unmodifiable views to the underlying data structure.

For these methods on BundleWiring we should change them to return a modifiable copy of the list.  The following methods on ModuleWiring can continue to return an unmodifiable list:

org.eclipse.osgi.container.ModuleWiring.getModuleCapabilities(String)
org.eclipse.osgi.container.ModuleWiring.getModuleRequirements(String)
org.eclipse.osgi.container.ModuleWiring.getProvidedModuleWires(String)
org.eclipse.osgi.container.ModuleWiring.getRequiredModuleWires(String)

We should also ensure that all internal usage of the wiring API by the framework uses the ModuleWiring methods to avoid extra copying.  Unfortunately the felix resolver must continue to use the general Wiring API to avoid requiring ModuleWiring or even BundleWiring during resolution.
Comment 1 Thomas Watson CLA 2021-08-23 12:35:39 EDT 
I'm working on a fix for this for RC1.
Comment 2 Thomas Watson CLA 2021-08-23 12:37:06 EDT 
FYI, I expect a clarification to come in relation to this from OSGi issue: https://github.com/osgi/osgi/issues/175
Comment 3 Eclipse Genie CLA 2021-08-23 12:43:14 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/184329
Comment 4 Hannes Wellmann CLA 2021-08-23 13:06:18 EDT 
OK, that is is too bad.
Is there a reason why it is now required that the returned list is mutable?
Do you think the specification gets more precise in the future and the returned list can become immutable again?
Comment 6 Hannes Wellmann CLA 2021-08-23 16:36:34 EDT 
@Thomas do you agree to BJ Hargrave's suggestion on the GitHub issue? https://github.com/osgi/osgi/issues/175#issuecomment-903978287

If yes, I can offer to implement it in a follow-up gerrit or bug into InternalUtils.asCopy(). Using a AbstractList, this could be done quite easily.
Comment 7 Thomas Watson CLA 2021-08-23 17:01:35 EDT 
(In reply to Hannes Wellmann from comment #6)
> @Thomas do you agree to BJ Hargrave's suggestion on the GitHub issue?
> https://github.com/osgi/osgi/issues/175#issuecomment-903978287
> 
> If yes, I can offer to implement it in a follow-up gerrit or bug into
> InternalUtils.asCopy(). Using a AbstractList, this could be done quite
> easily.

Yes, I had discussed that with BJ in the review.  This is why I created the method org.eclipse.osgi.internal.container.InternalUtils.asCopy(List<T>) so we could just place the specialized list there without impacting the rest of the code.  But please open a separate defect for that.  I think we would hold off and do that in 4.22.
Comment 8 Hannes Wellmann CLA 2021-08-23 17:16:32 EDT 
(In reply to Thomas Watson from comment #7)
>
> Yes, I had discussed that with BJ in the review.  This is why I created the
> method org.eclipse.osgi.internal.container.InternalUtils.asCopy(List<T>) so
> we could just place the specialized list there without impacting the rest of
> the code. But please open a separate defect for that. I think we would
> hold off and do that in 4.22.

Ok, perfect.
I'll create a separate bug.
Comment 9 Eclipse Genie CLA 2021-08-27 11:36:35 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/184541