jbonofre commented on code in PR #739:
URL: https://github.com/apache/camel-karaf/pull/739#discussion_r3872608021
##########
core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiTypeConverter.java:
##########
@@ -279,28 +358,73 @@ public Set<ClassLoader> getClassLoaders() {
throw new RuntimeCamelException("Error loading CoreTypeConverter
due: " + e.getMessage(), e);
}
- // Load the type converters the tracker has been tracking
- // Here we need to use the ServiceReference to check the ranking
- ServiceReference<TypeConverterLoader>[] serviceReferences =
this.tracker.getServiceReferences();
- if (serviceReferences != null) {
- ArrayList<ServiceReference<TypeConverterLoader>> servicesList =
- new ArrayList<>(Arrays.asList(serviceReferences));
- // Just make sure we install the high ranking fallback converter
at last
- Collections.sort(servicesList);
- for (ServiceReference<TypeConverterLoader> sr : servicesList) {
- try {
- LOG.debug("loading type converter from bundle: {}",
sr.getBundle().getSymbolicName());
-
((TypeConverterLoader)this.tracker.getService(sr)).load(answer);
- } catch (Throwable t) {
- throw new RuntimeCamelException("Error loading type
converters from service: " + sr + " due: " + t.getMessage(), t);
- }
+ // Load the type converters the tracker has been tracking. These come
from our own map rather than from
+ // tracker.getServiceReferences()/getService(): this runs while
holding this instance's monitor, and
+ // calling back into the tracker from here is what would establish a
lock ordering against the framework.
+ List<ServiceReference<TypeConverterLoader>> servicesList = new
ArrayList<>(trackedLoaders.keySet());
+ // Just make sure we install the high ranking fallback converter at
last
+ Collections.sort(servicesList);
+ for (ServiceReference<TypeConverterLoader> sr : servicesList) {
+ TypeConverterLoader loader = trackedLoaders.get(sr);
+ if (loader == null) {
+ // unregistered between the snapshot and here
+ continue;
+ }
+ try {
+ LOG.debug("loading type converter from bundle: {}",
sr.getBundle().getSymbolicName());
+ loader.load(answer);
+ } catch (Throwable t) {
+ throw new RuntimeCamelException("Error loading type converters
from service: " + sr + " due: " + t.getMessage(), t);
}
}
+ replayProgrammaticRegistrations(answer);
Review Comment:
**The replay is outside any try/catch, so one throwing registration
permanently bricks the registry — and with `TypeConverterExists.Fail` it is
guaranteed to throw.**
I disassembled `CoreTypeConverterRegistry` from `camel-base-4.18.1.jar`:
- `addConverter(TypeConvertible, TypeConverter)` is a bare
`converters.put(...)` — no duplicate check.
- `addTypeConverter(Class, Class, TypeConverter)` routes through
`addOrReplaceTypeConverter()` -> `onTypeConverterExists()`, which returns
`true` for `Override`, `false` for `Ignore`, and otherwise **throws
`TypeConverterExistsException`**.
So:
1. Context is configured `typeConverterExists=Fail`.
2. A Blueprint bean calls `addTypeConverter(Foo, Bar, tc)` while only core
converters are loaded — no conflict, applied and recorded.
3. Bundle X's `TypeConverterLoader` later registers `Bar -> Foo` via
`addConverter()` (plain `put`, no check).
4. Any loader unregisters; the delegate is discarded.
5. Next `getDelegate()`: loaders load **first**, then this replay calls
`addTypeConverter(Foo, Bar, tc)` -> existing converter found ->
`TypeConverterExistsException` escapes `createRegistry()` and `getDelegate()`.
`delegate` stays `null`, so every `convertTo` / `tryConvertTo` / `lookup` in
the container throws forever, and each retry re-throws because the offending
entry is still in `programmaticRegistrations`. The loader loop just above is
wrapped in `try/catch (Throwable)`; the replay is not.
Even on the default path this is lossy rather than fatal: the constructor
sets `typeConverterExists = Ignore` and `typeConverterExistsLoggingLevel =
LoggingLevel.DEBUG`, so the same collision silently drops the programmatic
converter and logs only at DEBUG.
`programmaticConverterSurvivesARegistryRebuild` does not catch it because the
`Marker` interface is chosen specifically so the pair cannot collide with a
core converter.
--
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]
