jbonofre commented on code in PR #739:
URL: https://github.com/apache/camel-karaf/pull/739#discussion_r3872608057
##########
core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiTypeConverter.java:
##########
@@ -173,27 +242,33 @@ public <T> T tryConvertTo(Class<T> type, Object value) {
@Override
public void addTypeConverter(Class<?> toType, Class<?> fromType,
TypeConverter typeConverter) {
- getDelegate().addTypeConverter(toType, fromType, typeConverter);
+ register(new TypeConvertible<>(fromType, toType),
+ registry -> registry.addTypeConverter(toType, fromType,
typeConverter));
}
@Override
public void addTypeConverters(Object typeConverters) {
- getDelegate().addTypeConverters(typeConverters);
+ register(typeConverters, registry ->
registry.addTypeConverters(typeConverters));
}
@Override
public void addBulkTypeConverters(BulkTypeConverters bulkTypeConverters) {
- getDelegate().addBulkTypeConverters(bulkTypeConverters);
+ register(bulkTypeConverters, registry ->
registry.addBulkTypeConverters(bulkTypeConverters));
}
@Override
- public boolean removeTypeConverter(Class<?> toType, Class<?> fromType) {
- return getDelegate().removeTypeConverter(toType, fromType);
+ public synchronized boolean removeTypeConverter(Class<?> toType, Class<?>
fromType) {
+ boolean removed = getDelegate().removeTypeConverter(toType, fromType);
+ // delete the matching registration rather than recording an inverse.
An inverse would reproduce the same
+ // end state on replay, but it would also keep the removed converter -
and its bundle's classloader -
+ // strongly reachable for the life of the context, and cost a no-op
call on every future rebuild
+ programmaticRegistrations.remove(new TypeConvertible<>(fromType,
toType));
+ return removed;
}
@Override
public void addFallbackTypeConverter(TypeConverter typeConverter, boolean
canPromote) {
- getDelegate().addFallbackTypeConverter(typeConverter, canPromote);
+ register(typeConverter, registry ->
registry.addFallbackTypeConverter(typeConverter, canPromote));
Review Comment:
**Replaying after the loader loop inverts fallback converter precedence,
because `addFallbackTypeConverter` prepends.**
`CoreTypeConverterRegistry.addFallbackTypeConverter` (verified in bytecode)
is:
```java
fallbackConverters.add(0, new FallbackTypeConverter(typeConverter,
canPromote));
```
i.e. **last added == index 0 == tried first**. That is what
`createRegistry()`'s "Just make sure we install the high ranking fallback
converter at last" comment relies on: the highest-ranking `ServiceReference`
sorts last, so its fallback lands at index 0.
`replayProgrammaticRegistrations(answer)` then runs *after* the entire loop,
so any programmatic fallback is prepended in front of the high-ranking loader
fallback that the sort deliberately put there.
In the live registry the opposite ordering held: a fallback registered at
time T sat behind every loader that arrived after T (`addingService` ->
`loader.load(delegate)` -> prepend). So a single loader unregistration silently
flips which fallback wins for every unmatched conversion in the context. The
field Javadoc's claim that the rebuilt registry is brought "back to the same
state" does not hold for fallbacks.
--
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]
