Community
Participate
Working Groups
I came across a in the KeyedHashSet as it is used in the BundleLoader. Following is a stacktrace and a detailed explanation of the situation: java.lang.NullPointerException at org.eclipse.osgi.framework.util.KeyedHashSet.hash(KeyedHashSet.java:376) at org.eclipse.osgi.framework.util.KeyedHashSet.add(KeyedHashSet.java:83) at org.eclipse.osgi.internal.loader.BundleLoader.findRequiredSource(BundleLoader.java:1214) at org.eclipse.osgi.internal.loader.BundleLoader.findSource(BundleLoader.java:1153) at org.eclipse.osgi.internal.loader.BundleLoader.getPackageSource(BundleLoader.java:1225) Details: It seems that this problem occurs in case of substituted exports. My setup is a little bit complicated but the situation is as follows. There are two bundles, A and B. A exports a package p, B imports and exports p, and contains p itself. So for the import of p in B there are two options for wiring - to A and to B. The export of p in B is marked as substituted for the import from A. There is bundle C which requires B and uses p from it. Finally, the resolution mechanism resolves the import to B's own export. When the BundleLoader is initialized (populated), the method BundleLoader.getImportedSources() is called. In this method for each import, which is resolved to an export from another bundle a PackageSource object is created and stored in a set. Since B imports p from itself, no PackageSource is created for p, although it is marked as substituted. The NPE occurs when later findRequiredSource() is called for package p on BundleLoader of bundle C. It calls addExportedProvidersFor() on BundleLoader of B. In addExportedProvidersFor() on line 975 it is checked if p is substituted export. Since it is, findImportSource() is called for p. In findImportdedSource() on line 1161 the set of PackageSources previously created is searched for package p. Since p is initially not included in this set, the result is null. Consequently, in method addExportedProvidersFor() a null value is added to the result list. findRequiredSource() gets the null value from the result list, and on line 1214 it tries to add it to a KeyedHashSet. The NPE is thrown when KeyedHashSet tries to calculate the hash value for null. A possible fix could be the getImportedSouces() method to consider substituted exports when initializes the set of package sources, so that if an export is substituted, it gets added to the set. But I am not sure if this won't break some other logic. I haven't yet managed to reproduce this issue with a simple setup. However, if I manage I will attache sample bundles to the bug.
(In reply to comment #0) > There are two bundles, A and B. A exports a package p, B imports and exports p, > and contains p itself. > So for the import of p in B there are two options for wiring - to A and to B. > The export of p in B is marked as substituted for the import from A. > There is bundle C which requires B and uses p from it. When you say "bundle C which requires B and uses p from it", do you mean C's manifest has Import-Package: p;bundle-symbolic-name="b" or Require-Bundle: b?
The case is Require-Bundle: b
(In reply to comment #0) > The export of p in B is marked as substituted for the import from A. > ... > Finally, the resolution mechanism resolves the import to B's own export. It seems to me the real problem lies here. From 3.13.1 of the core spec: "However, if a required bundle's exported package is substituted for an imported package, then the requiring bundles must get wired to the same exported package that the required bundle is wired to ensure class space consistency." And it sounds like this is exactly what's not happening. It also seems like the proposed fix would break this logic, as you surmised might be the case. We don't want B to have a substituted export for p and C end up getting wired to B's export of p when it should be wired to A's export of p. How are you identifying that the "export of p in B is marked as substituted for the import from A"? Are you seeing this in the console or did you actually see in the debugger that there was a BundleLoader instance that had a "p" entry in the substitutedPackages variable as well as an ExportPackageDescription from its BundleDescription.getResolvedImports() whose exporter was B? The substitutedPackages variable is initialized in the BundleLoader constructor using the ExportPackageDescriptions from BundleDescription.getSubstitutedExports. In BundleLoader.getImportedSources, however, the ExportPackageDescription data is not cached but retrieved from the BundleDescription each time. Although the BundleLoader's BundleDescription instance reference never changes, it looks like the BundleDescription is designed to be updated. If so, the cached substituted packages data and the dynamic resolved imports data could get out of sync.
(In reply to comment #3) > The substitutedPackages variable is initialized in the BundleLoader constructor > using the ExportPackageDescriptions from > BundleDescription.getSubstitutedExports. In BundleLoader.getImportedSources, > however, the ExportPackageDescription data is not cached but retrieved from the > BundleDescription each time. Although the BundleLoader's BundleDescription > instance reference never changes, it looks like the BundleDescription is > designed to be updated. If so, the cached substituted packages data and the > dynamic resolved imports data could get out of sync. Actually, the truth is that the resolved imports data is cached in the importedSources variable but lazily instantiated. There's still a window of opportunity between when the constructor is called and when BundleLoader.getImportedSources is called for the first time.
(In reply to comment #3) > How are you identifying that the "export of p in B is marked as substituted for > the import from A"? Are you seeing this in the console or did you actually see > in the debugger that there was a BundleLoader instance that had a "p" entry in > the substitutedPackages variable as well as an ExportPackageDescription from > its BundleDescription.getResolvedImports() whose exporter was B? Yes, I see this in the debugger.
Then you mean that BundleLoader.findSource() should find package p with the call to findImportedSources() and not get to call findRequiredSources() at all? Because from the code it seems to me that if the flow reaches findRequiredSources(), we will always get the NPE.
(In reply to comment #6) > Then you mean that BundleLoader.findSource() should find package p with the > call to findImportedSources() and not get to call findRequiredSources() at all? Essentially, yes. > Because from the code it seems to me that if the flow reaches > findRequiredSources(), we will always get the NPE. Yes, assuming the BundleLoader has the state being described here, which is substitutedPackages contains "p" because B's export of p has been substituted for B's import of p from A and proxy.getBundleDescription().getResolvedImports() contains an ExportPackageDescription for "p" whose exporter is B, then an NPE will always result. Primarily, however, I'm suggesting this state is invalid and should never occur in the first place. If B's export has been substituted for A's export of p, B's resolved import for p should have exporter A. Are any of the bundles involved here (A, B, or C) fragments?
(In > > Are any of the bundles involved here (A, B, or C) fragments? No, neither is a fragment. Actually, in the setup there are more bundles, all with a large number of imported/exported packages and there are uses constraints, including for package p. I suppose that this causes finally B to get p from itself and not from A.
Created attachment 208075 [details] possible fix and testcase (In reply to comment #8) > (In > > > > Are any of the bundles involved here (A, B, or C) fragments? > > No, neither is a fragment. Actually, in the setup there are more bundles, all > with a large number of imported/exported packages and there are uses > constraints, including for package p. I suppose that this causes finally B to > get p from itself and not from A. I was able to reproduce this with a small scenario. It does involve the "uses" directive. Here is the basic scenario: Bundle A: Export-Package: x; version=1.0, z; version=1.1; uses:=x Bundle B: Export-Package: x; y; version=1.0; uses:="x,y" Bundle C: Export-Package: z; version=1.0 Require-Bundle: B Import-Package: z Bundle D: Require-Bundle: C In this scenario the problematic bundle is C which has substitutable export package 'z'. The resolver prefers the export A.z because it is a higher version (1.1) so it attempts to substitute the export of C.z with A.z. But then the uses consistency check is done which determines that for bundle C the import of A.z conflicts with the require bundle B because B.y uses B.x and A.z uses A.x so B.x conflicts with A.x for bundle C making an inconsistent class space for bundle C. The resolver then determines that bundle C can actually resolve to its own export C.z BUT ... it neglects to unmark the export C.z as substituted by A.z. This results in some really bogus results from the Resolver to the State. The State will see that the import C.z got wired to the export C.z but the resolver also tells the State that C.z was not selected as an export and it got substituted when really C.z was selected and NOT substituted. Attached is a possible fix and testcase. Lazar could you try this patch out in your scenario? Thanks for your patients and analysis so far!
(In reply to comment #9) > Attached is a possible fix and testcase. Lazar could you try this patch out in > your scenario? Thanks for your patients and analysis so far! Thanks a lot, Tom. I tried the fix and is working fine. No NPE occurs now.
Fix released: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=454e06e626bd751a146b10ae01bbe3885a1d739c Will be in the next integration build for Juno M5.
