This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch camel-karaf-4.18.x
in repository https://gitbox.apache.org/repos/asf/camel-karaf.git
The following commit(s) were added to refs/heads/camel-karaf-4.18.x by this
push:
new 2f7c6142b fix(#733): do not let an uninstalled bundle poison factory
resolution (backport of #741) (#752)
2f7c6142b is described below
commit 2f7c6142bf893a4128c21e3cc6023e74721f7e70
Author: JB Onofré <[email protected]>
AuthorDate: Mon Sep 7 11:00:31 2026 +0200
fix(#733): do not let an uninstalled bundle poison factory resolution
(backport of #741) (#752)
getResource walks bundleContext.getBundles() and returns the first bundle
with
a matching descriptor. getBundles() returns a snapshot, and per the OSGi
spec
Bundle.getEntry() throws IllegalStateException once a bundle has been
uninstalled, so a concurrent feature:uninstall or bundle:update can make the
scan throw on a bundle that has nothing to do with the factory being
resolved.
findClass calls getResource from inside DefaultFactoryFinder.addToClassMap,
which catches Exception, stores it in classesNotFoundExceptions and
rethrows it
on every later lookup of the same key. That turns the transient race into a
lasting failure: the key stays broken for the life of the context, until
clear(). Skip bundles already in state UNINSTALLED, and treat an
IllegalStateException from getEntry as "this bundle has no such entry"
rather
than letting it fail the whole scan.
Selection is unchanged. The scan still stops at the first match, so install
order still decides and there is no added cost on a path that is walked once
per key. Log the bundle that supplied the descriptor at DEBUG: with several
providers installed, that is what lets an operator tell which one is
actually
in use.
Deliberately not warning when several bundles provide the same descriptor.
The
default camel-core feature installs both camel-xml-io and camel-xml-jaxb,
and
both ship META-INF/services/org/apache/camel/modelxml-dumper with a
different
implementation class, so a stock install would warn on every startup. That
ambiguity is inherited from flat-classpath Camel, where classpath order
picks
the winner in the same way.
Not addressed here: the rolling-upgrade half of #733. Once a key resolves,
addToClassMap short-circuits on classMap and getResource is never called
again
for it, so a patched bundle installed afterwards is still silently ignored
by
an already-running context. Catching that needs cache invalidation on bundle
events, in the spirit of OsgiTypeConverter revalidating its delegate on
service
changes, and is a separate change with its own blast radius.
BundleEntry becomes package private. It was private while getResource, which
returns it, is public, so the modifier was not restricting anything; this
makes
it reachable from the tests in the package.
Co-authored-by: Andrea Cosentino <[email protected]>
---
.../apache/camel/karaf/core/OsgiFactoryFinder.java | 40 ++++-
.../camel/karaf/core/OsgiFactoryFinderTest.java | 178 +++++++++++++++++++++
2 files changed, 214 insertions(+), 4 deletions(-)
diff --git
a/core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java
b/core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java
index 166139281..ff4b167e0 100644
---
a/core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java
+++
b/core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java
@@ -28,8 +28,12 @@ import org.apache.camel.spi.ClassResolver;
import org.apache.camel.util.IOHelper;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class OsgiFactoryFinder extends DefaultFactoryFinder {
+ private static final Logger LOG =
LoggerFactory.getLogger(OsgiFactoryFinder.class);
+
private final BundleContext bundleContext;
public OsgiFactoryFinder(BundleContext bundleContext, ClassResolver
classResolver, String resourcePath) {
@@ -37,7 +41,9 @@ public class OsgiFactoryFinder extends DefaultFactoryFinder {
this.bundleContext = bundleContext;
}
- private static class BundleEntry {
+ // package private rather than private: getResource is public and returns
it, so private was never
+ // actually restricting anything, and it keeps the type reachable from the
tests in this package
+ static class BundleEntry {
URL url;
Bundle bundle;
}
@@ -83,9 +89,24 @@ public class OsgiFactoryFinder extends DefaultFactoryFinder {
bundles = bundleContext.getBundles();
+ String path = getResourcePath() + name;
URL url;
for (Bundle bundle : bundles) {
- url = bundle.getEntry(getResourcePath() + name);
+ // getBundles() is a snapshot, so a bundle in it can be
uninstalled by the time we get here,
+ // for instance by a concurrent feature:uninstall or
bundle:update. getEntry then throws
+ // IllegalStateException, and findClass calls us from inside
addToClassMap, which caches the
+ // failure in classesNotFoundExceptions and rethrows it for every
later lookup of this key.
+ // A bundle unrelated to the factory would poison the key for the
life of the context, so
+ // skip such a bundle instead of letting it fail the whole scan.
+ if (bundle.getState() == Bundle.UNINSTALLED) {
+ continue;
+ }
+ try {
+ url = bundle.getEntry(path);
+ } catch (IllegalStateException e) {
+ // uninstalled between the state check and here
+ continue;
+ }
if (url != null) {
entry = new BundleEntry();
entry.url = url;
@@ -94,8 +115,19 @@ public class OsgiFactoryFinder extends DefaultFactoryFinder
{
}
}
+ if (LOG.isDebugEnabled()) {
+ // which bundle wins is the container's install order, and
findClass caches the answer per key,
+ // so record the choice: it is the only way an operator can tell
which bundle actually supplies
+ // a factory when several provide the same descriptor
+ if (entry == null) {
+ LOG.debug("Factory descriptor {} is not provided by any
installed bundle", path);
+ } else {
+ LOG.debug("Factory descriptor {} resolved from bundle {}/{}
[{}]", path,
+ entry.bundle.getSymbolicName(),
entry.bundle.getVersion(), entry.bundle.getBundleId());
+ }
+ }
+
return entry;
}
-
-}
\ No newline at end of file
+}
diff --git
a/core/camel-core-osgi/src/test/java/org/apache/camel/karaf/core/OsgiFactoryFinderTest.java
b/core/camel-core-osgi/src/test/java/org/apache/camel/karaf/core/OsgiFactoryFinderTest.java
new file mode 100644
index 000000000..104fa4514
--- /dev/null
+++
b/core/camel-core-osgi/src/test/java/org/apache/camel/karaf/core/OsgiFactoryFinderTest.java
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.karaf.core;
+
+import java.net.URI;
+import java.net.URL;
+
+import org.apache.camel.spi.ClassResolver;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+public class OsgiFactoryFinderTest {
+
+ private static final String PATH = "META-INF/services/org/apache/camel/";
+ private static final String KEY = "some-factory";
+
+ @Mock
+ private BundleContext bundleContext;
+ @Mock
+ private ClassResolver classResolver;
+
+ private OsgiFactoryFinder finder() {
+ return new OsgiFactoryFinder(bundleContext, classResolver, PATH);
+ }
+
+ private static URL url(String spec) {
+ try {
+ return URI.create(spec).toURL();
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ /**
+ * Builds the bundle mocks eagerly, before the {@code
when(bundleContext.getBundles())} that hands them
+ * out is opened: stubbing a mock while an outer stubbing is still pending
makes Mockito report
+ * UnfinishedStubbing.
+ * <p>
+ * Stubbed leniently because the scan stops at the first provider, so a
bundle placed after it is
+ * legitimately never consulted.
+ */
+ private Bundle bundle(URL entry) {
+ Bundle bundle = mock(Bundle.class);
+ lenient().when(bundle.getState()).thenReturn(Bundle.ACTIVE);
+ lenient().when(bundle.getEntry(PATH + KEY)).thenReturn(entry);
+ return bundle;
+ }
+
+ private Bundle uninstalledBundle() {
+ Bundle bundle = mock(Bundle.class);
+ lenient().when(bundle.getState()).thenReturn(Bundle.UNINSTALLED);
+ return bundle;
+ }
+
+ /**
+ * A bundle that has been uninstalled since {@code getBundles()}
snapshotted it, but is still reporting
+ * a live state: {@code getEntry} is specified to throw for an uninstalled
bundle.
+ */
+ private Bundle bundleUninstalledMidScan() {
+ Bundle bundle = mock(Bundle.class);
+ lenient().when(bundle.getState()).thenReturn(Bundle.ACTIVE);
+ lenient().when(bundle.getEntry(PATH + KEY)).thenThrow(new
IllegalStateException("bundle is uninstalled"));
+ return bundle;
+ }
+
+ @Test
+ public void testReturnsNullWhenNoBundleProvidesTheDescriptor() {
+ Bundle a = bundle(null);
+ Bundle b = bundle(null);
+ when(bundleContext.getBundles()).thenReturn(new Bundle[] {a, b});
+
+ assertNull(finder().getResource(KEY));
+ }
+
+ @Test
+ public void testReturnsTheOnlyProvider() {
+ URL url = url("file:///a");
+ Bundle only = bundle(url);
+ Bundle other = bundle(null);
+ when(bundleContext.getBundles()).thenReturn(new Bundle[] {only,
other});
+
+ OsgiFactoryFinder.BundleEntry entry = finder().getResource(KEY);
+ assertNotNull(entry);
+ assertSame(only, entry.bundle);
+ assertSame(url, entry.url);
+ }
+
+ /**
+ * Selection is first-match-wins on the container's bundle install order.
That is the behaviour the rest
+ * of the resolution path depends on, and findClass caches it per key, so
pin it against future edits.
+ */
+ @Test
+ public void testFirstProviderWinsWhenSeveralProvideTheDescriptor() {
+ URL first = url("file:///first");
+ URL second = url("file:///second");
+ Bundle winner = bundle(first);
+ Bundle loser = bundle(second);
+ when(bundleContext.getBundles()).thenReturn(new Bundle[] {winner,
loser});
+
+ OsgiFactoryFinder.BundleEntry entry = finder().getResource(KEY);
+ assertNotNull(entry);
+ assertSame(winner, entry.bundle, "install order decides, and the
choice is cached by findClass");
+ assertSame(first, entry.url);
+ }
+
+ /**
+ * The scan must not ask an uninstalled bundle for an entry: it is
specified to throw
+ * IllegalStateException, which DefaultFactoryFinder.addToClassMap would
cache in
+ * classesNotFoundExceptions and rethrow for every later lookup of the
same key.
+ */
+ @Test
+ public void testSkipsUninstalledBundle() {
+ Bundle uninstalled = uninstalledBundle();
+ URL url = url("file:///a");
+ Bundle provider = bundle(url);
+ when(bundleContext.getBundles()).thenReturn(new Bundle[] {uninstalled,
provider});
+
+ OsgiFactoryFinder.BundleEntry entry = finder().getResource(KEY);
+ assertNotNull(entry);
+ assertSame(provider, entry.bundle);
+ verify(uninstalled, never()).getEntry(anyString());
+ }
+
+ /**
+ * Same race, but lost between the state check and the getEntry call: a
bundle uninstalled concurrently
+ * must not fail the lookup for every other bundle.
+ */
+ @Test
+ public void testBundleUninstalledMidScanDoesNotFailTheLookup() {
+ Bundle racing = bundleUninstalledMidScan();
+ URL url = url("file:///a");
+ Bundle provider = bundle(url);
+ when(bundleContext.getBundles()).thenReturn(new Bundle[] {racing,
provider});
+
+ OsgiFactoryFinder.BundleEntry entry = finder().getResource(KEY);
+ assertNotNull(entry);
+ assertSame(provider, entry.bundle);
+ assertSame(url, entry.url);
+ }
+
+ @Test
+ public void
testBundleUninstalledMidScanStillReturnsNullWhenNobodyProvides() {
+ Bundle racing = bundleUninstalledMidScan();
+ Bundle other = bundle(null);
+ when(bundleContext.getBundles()).thenReturn(new Bundle[] {racing,
other});
+
+ assertNull(finder().getResource(KEY));
+ }
+}
