gnodet-bot commented on code in PR #28652:
URL: https://github.com/apache/camel/pull/28652#discussion_r4408682377
##########
core/camel-base/src/main/java/org/apache/camel/impl/converter/CoreTypeConverterRegistry.java:
##########
@@ -587,7 +606,19 @@ public void addFallbackTypeConverter(TypeConverter
typeConverter, boolean canPro
}
public TypeConverter lookup(Class<?> toType, Class<?> fromType) {
- return doLookup(toType, fromType);
+ final TypeConvertible<?, ?> typeConvertible = new
TypeConvertible<>(fromType, toType);
+ TypeConverter answer = lookups.get(typeConvertible);
+ if (answer == null) {
+ final long version = lookupsVersion.get();
+ final TypeConverter found = doLookup(toType, fromType);
+ answer = found != null ? found : MISS_CONVERTER;
+ lookups.put(typeConvertible, answer);
+ if (lookupsVersion.get() != version) {
+ // the converters changed while looking up, so the result may
be stale
+ lookups.remove(typeConvertible, answer);
Review Comment:
💡 **Documentation suggestion:** Consider adding a comment explaining why the
two-arg conditional form is used here rather than the single-arg
`lookups.remove(typeConvertible)`:
```suggestion
// Use the two-arg conditional remove: if another thread
concurrently added a fresher
// entry for this key, the single-arg remove would delete
it. The conditional form
// only removes the value if it still matches what we put
there, leaving any newer
// entry from another thread intact.
lookups.remove(typeConvertible, answer);
```
The code is correct — this is purely to document the concurrency safety
rationale for future readers.
--
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]
