jbonofre commented on code in PR #741:
URL: https://github.com/apache/camel-karaf/pull/741#discussion_r3907733241
##########
core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java:
##########
@@ -79,23 +87,51 @@ public Optional<Class<?>> findClass(String key) {
// NOTE, the first found factory will be return
public BundleEntry getResource(String name) {
BundleEntry entry = null;
- Bundle[] bundles;
+ // only allocated when more than one bundle provides the same
descriptor, which is not the normal case
+ List<Bundle> alsoProviding = null;
- bundles = bundleContext.getBundles();
+ Bundle[] bundles = bundleContext.getBundles();
- URL url;
for (Bundle bundle : bundles) {
- url = bundle.getEntry(getResourcePath() + name);
+ URL url = bundle.getEntry(getResourcePath() + name);
if (url != null) {
- entry = new BundleEntry();
- entry.url = url;
- entry.bundle = bundle;
- break;
+ if (entry == null) {
+ entry = new BundleEntry();
+ entry.url = url;
+ entry.bundle = bundle;
+ } else {
+ if (alsoProviding == null) {
+ alsoProviding = new ArrayList<>();
+ }
+ alsoProviding.add(bundle);
+ }
}
}
+ if (alsoProviding != null) {
Review Comment:
I have not tried to patch around it: no logging inside `getResource` can see
that sequence, because step 3 never reaches `getResource` at all. Confirmed in
the bytecode: `classMap.computeIfAbsent`, so a resolved key never re-scans.
So the PR no longer claims to detect it. What it keeps is the cheap, honest
part: a DEBUG line naming the bundle that supplied each descriptor, which
answers "which one is actually in use" for a context that resolves after the
fact.
I have left #733 open for the sticky-cache half. Doing it properly means
invalidating on bundle events the way `OsgiTypeConverter` revalidates its
delegate — with the wrinkle that `OsgiFactoryFinder` has no dispose hook today
(it is created per resource path by `OsgiFactoryFinderResolver` and never torn
down), so a `BundleListener` registered from it would leak. That wants its own
PR.
##########
core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java:
##########
@@ -79,23 +87,51 @@ public Optional<Class<?>> findClass(String key) {
// NOTE, the first found factory will be return
public BundleEntry getResource(String name) {
BundleEntry entry = null;
- Bundle[] bundles;
+ // only allocated when more than one bundle provides the same
descriptor, which is not the normal case
+ List<Bundle> alsoProviding = null;
- bundles = bundleContext.getBundles();
+ Bundle[] bundles = bundleContext.getBundles();
- URL url;
for (Bundle bundle : bundles) {
- url = bundle.getEntry(getResourcePath() + name);
+ URL url = bundle.getEntry(getResourcePath() + name);
if (url != null) {
- entry = new BundleEntry();
- entry.url = url;
- entry.bundle = bundle;
- break;
+ if (entry == null) {
+ entry = new BundleEntry();
+ entry.url = url;
+ entry.bundle = bundle;
+ } else {
+ if (alsoProviding == null) {
+ alsoProviding = new ArrayList<>();
+ }
+ alsoProviding.add(bundle);
+ }
}
}
+ if (alsoProviding != null) {
Review Comment:
I have not tried to patch around it: no logging inside `getResource` can see
that sequence, because step 3 never reaches `getResource` at all. Confirmed in
the bytecode: `classMap.computeIfAbsent`, so a resolved key never re-scans.
So the PR no longer claims to detect it. What it keeps is the cheap, honest
part: a DEBUG line naming the bundle that supplied each descriptor, which
answers "which one is actually in use" for a context that resolves after the
fact.
I have left #733 open for the sticky-cache half. Doing it properly means
invalidating on bundle events the way `OsgiTypeConverter` revalidates its
delegate: with the wrinkle that `OsgiFactoryFinder` has no dispose hook today
(it is created per resource path by `OsgiFactoryFinderResolver` and never torn
down), so a `BundleListener` registered from it would leak. That wants its own
PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
