Bug 575576 - BundleWiring and Wiring lists must be snapshot/mutable
Summary: BundleWiring and Wiring lists must be snapshot/mutable
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Framework (show other bugs)
Version: 4.21   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 4.21 RC1   Edit
Assignee: Thomas Watson CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-08-23 12:33 EDT by Thomas Watson CLA
Modified: 2021-08-30 09:22 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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