Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 324618

Summary:

Dynamic import resolver bug

Product:

[Eclipse Project] Equinox

Reporter:

Glyn Normington <glyn.normington>

Component:

Framework

Assignee:

Thomas Watson <tjwatson>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

anbharga, jeffmcaffer

Version:

3.5.1

Target Milestone:

3.7 M2

Hardware:

OS:

Mac OS X - Carbon (unsup.)

Whiteboard:

Bug Depends on:

Bug Blocks:

325883, 325885

Attachments:

Description	Flags
testcase	none
possible fix	none
Patch against 3.5.1	none

Description Glyn Normington

2010-09-07 03:31:50 EDT

A customer has observed some strange behaviour in the Equinox resolver (version 3.5.1.R35x_v20091005) and has spotted some code which may be to blame.

ResolverImpl line 1561 is:

resolverImports[j].setPossibleSuppliers(null))

rather than:

resolverImports[j].clearPossibleSuppliers()

and so possibleSuppliers in the array member is set to null, but selectedSupplierIndex is not set to 0 as well.

They say that, upon debugging, they ran into an issue where, in ResolverConstraint.selectNextSupplier(), the selectedSupplierIndex is not reset to 0 and, even if possibleSuppliers has values, selectNextSupplier returns false because selectedSupplierIndex is 1 (because it was not reset in the past) and possibleSuppliers.length is 1.

We don't have a (simple) testcase to reproduce, or indeed any way of reproducing the behaviour the customer consistently observes.

Assigning to Tom as he has already started to look at this.

Comment 1 Thomas Watson

2010-09-07 13:08:08 EDT

Created attachment 178341 [details]
testcase

In trying to create a testcase for this I have uncovered a deeper more troubling dynamic import package bug.  The class space consistency checking does not take into account previously resolved dynamic imports when dynamically importing a new package.  Here is a testcase that shows the issue.

Comment 2 Thomas Watson

2010-09-07 13:57:42 EDT

Created attachment 178344 [details]
possible fix

Here is a possible fix.  The fix has two parts

1) In ResolverImpl.resolveDynamicImport(BundleDescription, String) we must call ResolverConstraint.clearPossibleSuppliers().  That is necessary and the analysis by your customer is correct.  Thanks!

2) But I found a more troubling bug that allowed for inconsistent class spaces to get introduced by the use of DynamicImport-Package.  The testcase attached tests this as well as the first issue.  To fix this we need to handle how we calculate the package roots for packages imported from already resolved bundles.  In the previous code we only checked the list of imports, but this is not correct because dynamic import * can add an endless number of exports to the package space.  We really need to call BundleDescription.getResolvedImports() to accurately get the package roots for already resolved bundles.

Comment 3 Glyn Normington

2010-09-07 14:01:04 EDT

Great progress Tom. How likely do you think it is that our customer's problem is also hitting the more complex aspect that you have uncovered? And what's the timescale for 3.7 M2.

Comment 4 Thomas Watson

2010-09-07 14:19:42 EDT

They would have seen issue related to class space consistency issues.  Things like cast exceptions or inability to access services because they are wired to the wrong package.

Comment 5 Jeff McAffer

2010-09-07 17:28:42 EDT

Created attachment 178361 [details]
Patch against 3.5.1

This is Tom's possible fix back ported to 3.5.1.  I can confirm that the testcase now passes.

Comment 6 Thomas Watson

2010-09-08 15:36:51 EDT

Patch released to head.

Comment 7 Ankur

2010-09-08 17:34:20 EDT

(In reply to comment #6)
> Patch released to head.

How do we get the jar file for 3.5.1 with this fix ?

Comment 8 Glyn Normington

2010-09-09 04:14:27 EDT

Thanks for providing this fix so promptly.

I have sent the fix to the customer and am awaiting their feedback. I will close this bug when they confirm their problem is fixed.

Comment 9 Jeff McAffer

2010-09-16 10:50:30 EDT

Looks like the changes fix the issue that was originally happening.  We should look to apply it in the 3.5 and 3.6 maintenance streams.