gnodet commented on code in PR #2428:
URL: https://github.com/apache/maven/pull/2428#discussion_r2125798341
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java:
##########
@@ -426,29 +425,14 @@ private void removeFromResolutionStack(Key<?> key) {
}
private static class SingletonScope implements Scope {
- Map<Key<?>, java.util.function.Supplier<?>> cache = new
ConcurrentHashMap<>();
+ final Map<Key<?>, java.util.function.Supplier<?>> cache = new
ConcurrentHashMap<>();
@Nonnull
@SuppressWarnings("unchecked")
@Override
public <T> java.util.function.Supplier<T> scope(
@Nonnull Key<T> key, @Nonnull java.util.function.Supplier<T>
unscoped) {
- return (java.util.function.Supplier<T>)
- cache.computeIfAbsent(key, k -> new
java.util.function.Supplier<T>() {
- volatile T instance;
-
- @Override
- public T get() {
- if (instance == null) {
- synchronized (this) {
- if (instance == null) {
- instance = unscoped.get();
- }
- }
- }
- return instance;
- }
- });
+ return (java.util.function.Supplier<T>) cache.computeIfAbsent(key,
k -> unscoped);
Review Comment:
Wow, don't change the logic here ! You're just completely breaking the
_singleton scope_ which would not memoize the values anymore. Your proposal
just returns the original `unscoped` supplier, so `unscoped.get()` will be
called every time the returned supplier is invoked, whereas the first one
memoize the value so that it's only called once.
--
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: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
