Skip to content

Commit 359464e

Browse files
committed
GROOVY-12280: hold cached awaitable adapters through SoftReferences
A ClassValue association lives as long as its key class, and the common keys here are platform classes such as CompletableFuture, so the cached Groovy-loaded adapter pinned Groovy's class loader for the lifetime of the JVM (JDK-8136353 behavior, working as intended). The value is now held through a SoftReference: the association strongly reaches only java.base objects. A cleared reference is removed and recomputed once; should the fresh reference already be cleared, the answer comes from an uncached scan, so lookups terminate under any memory pressure. A NO_ADAPTER sentinel keeps unsupported types distinguishable from cleared references. The site deliberately stays on java.lang.ClassValue rather than routing through GroovyClassValueFactory: with soft values nothing strongly Groovy-loaded remains in the association, so there is nothing left for the groovy.use.classvalue escape hatch to release here.
1 parent 1c2f0f2 commit 359464e

2 files changed

Lines changed: 162 additions & 19 deletions

File tree

src/main/java/groovy/concurrent/AwaitableAdapterRegistry.java

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.groovy.runtime.async.FlowPublisherAdapter;
2323
import org.apache.groovy.runtime.async.GroovyPromise;
2424

25+
import java.lang.ref.SoftReference;
2526
import java.util.Iterator;
2627
import java.util.List;
2728
import java.util.Objects;
@@ -51,7 +52,31 @@ public final class AwaitableAdapterRegistry {
5152

5253
private static final List<AwaitableAdapter> adapters = new CopyOnWriteArrayList<>();
5354

54-
private static volatile ClassValue<AwaitableAdapter> awaitableCache = buildAwaitableCache();
55+
/**
56+
* Marks a type no adapter supports, so that a cache entry whose {@link SoftReference}
57+
* was cleared can be told apart from a type that resolved to no adapter.
58+
*/
59+
private static final AwaitableAdapter NO_ADAPTER = new AwaitableAdapter() {
60+
@Override
61+
public boolean supportsAwaitable(Class<?> type) {
62+
return false;
63+
}
64+
65+
@Override
66+
public <T> Awaitable<T> toAwaitable(Object source) {
67+
throw new IllegalStateException("NO_ADAPTER cannot adapt");
68+
}
69+
};
70+
71+
/**
72+
* Adapter lookups cached per source class. A {@code ClassValue} association lives as
73+
* long as its key class, and the common keys here are platform classes such as
74+
* {@link CompletableFuture}, so the value is held through a {@link SoftReference}:
75+
* the association then strongly reaches only {@code java.base} objects and never pins
76+
* the adapter's class loader (GROOVY-12280). A cleared reference is re-resolved from
77+
* {@link #adapters} on the next lookup.
78+
*/
79+
private static volatile ClassValue<SoftReference<AwaitableAdapter>> awaitableCache = buildAwaitableCache();
5580

5681
static {
5782
// Load SPI adapters
@@ -104,7 +129,7 @@ static <T> Awaitable<T> toAwaitable(Object source) {
104129
}
105130
if (source instanceof Awaitable) return (Awaitable<T>) source;
106131
Class<?> type = source.getClass();
107-
AwaitableAdapter adapter = awaitableCache.get(type);
132+
AwaitableAdapter adapter = adapterFor(type);
108133
if (adapter != null) {
109134
return adapter.toAwaitable(source);
110135
}
@@ -113,6 +138,38 @@ static <T> Awaitable<T> toAwaitable(Object source) {
113138
+ ". Register an AwaitableAdapter via ServiceLoader or AwaitableAdapterRegistry.register().");
114139
}
115140

141+
/**
142+
* The adapter for the given type, or {@code null} when none supports it.
143+
* <p>
144+
* A cleared cache reference is removed and recomputed once; should the fresh
145+
* reference already be cleared as well, the answer comes from an uncached scan,
146+
* so the lookup terminates under any memory pressure.
147+
*/
148+
private static AwaitableAdapter adapterFor(Class<?> type) {
149+
ClassValue<SoftReference<AwaitableAdapter>> cache = awaitableCache;
150+
AwaitableAdapter adapter = cache.get(type).get();
151+
if (adapter == null) {
152+
cache.remove(type);
153+
adapter = cache.get(type).get();
154+
if (adapter == null) {
155+
adapter = resolveAdapter(type);
156+
}
157+
}
158+
return adapter == NO_ADAPTER ? null : adapter;
159+
}
160+
161+
/**
162+
* Resolves the first adapter supporting the supplied type, or {@link #NO_ADAPTER}.
163+
*/
164+
private static AwaitableAdapter resolveAdapter(Class<?> type) {
165+
for (AwaitableAdapter adapter : adapters) {
166+
if (adapter.supportsAwaitable(type)) {
167+
return adapter;
168+
}
169+
}
170+
return NO_ADAPTER;
171+
}
172+
116173
/**
117174
* Converts the given source to an {@link Iterable} for {@code for await}.
118175
*/
@@ -132,25 +189,14 @@ public static <T> Iterable<T> toIterable(Object source) {
132189
+ ". Register an AwaitableAdapter via ServiceLoader or AwaitableAdapterRegistry.register().");
133190
}
134191

135-
private static ClassValue<AwaitableAdapter> buildAwaitableCache() {
136-
/**
137-
* Cache of awaitable adapters by source type.
138-
*/
192+
// Keep this method static and the ClassValue below free of any enclosing-instance reference:
193+
// capturing one would let the association hold this registry, its class and its loader, undoing
194+
// the loader-unloading this class exists to allow (GROOVY-12280).
195+
private static ClassValue<SoftReference<AwaitableAdapter>> buildAwaitableCache() {
139196
return new ClassValue<>() {
140-
/**
141-
* Resolves the first adapter supporting the supplied type.
142-
*
143-
* @param type the source type
144-
* @return the matching adapter, or {@code null} if none match
145-
*/
146197
@Override
147-
protected AwaitableAdapter computeValue(Class<?> type) {
148-
for (AwaitableAdapter adapter : adapters) {
149-
if (adapter.supportsAwaitable(type)) {
150-
return adapter;
151-
}
152-
}
153-
return null;
198+
protected SoftReference<AwaitableAdapter> computeValue(Class<?> type) {
199+
return new SoftReference<>(resolveAdapter(type));
154200
}
155201
};
156202
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package groovy.concurrent
20+
21+
import org.junit.jupiter.api.Test
22+
23+
import java.lang.ref.SoftReference
24+
import java.util.concurrent.CompletableFuture
25+
26+
import static groovy.test.GroovyAssert.shouldFail
27+
28+
/**
29+
* GROOVY-12280: the adapter cache's {@code ClassValue} associations live as long as their
30+
* key classes, and the common keys are platform classes, so the cached adapter is held
31+
* through a {@link SoftReference} and re-resolved when the reference has been cleared.
32+
* These tests drive the cleared-reference protocol deterministically by clearing the
33+
* references directly rather than simulating memory pressure.
34+
*/
35+
final class AwaitableAdapterRegistryCacheTest {
36+
37+
private static ClassValue<SoftReference<?>> cache() {
38+
def field = AwaitableAdapterRegistry.getDeclaredField('awaitableCache')
39+
field.accessible = true
40+
field.get(null) as ClassValue<SoftReference<?>>
41+
}
42+
43+
@Test
44+
void testCachedValueIsHeldThroughASoftReference() {
45+
AwaitableAdapterRegistry.toAwaitable(CompletableFuture.completedFuture(1))
46+
def entry = cache().get(CompletableFuture)
47+
assert entry instanceof SoftReference
48+
assert entry.get() instanceof AwaitableAdapter
49+
}
50+
51+
@Test
52+
void testClearedReferenceIsReResolved() {
53+
def future = CompletableFuture.completedFuture(42)
54+
assert AwaitableAdapterRegistry.toAwaitable(future).get() == 42
55+
56+
cache().get(CompletableFuture).clear()
57+
58+
assert AwaitableAdapterRegistry.toAwaitable(future).get() == 42
59+
}
60+
61+
@Test
62+
void testUnsupportedTypeStillFailsAfterItsReferenceIsCleared() {
63+
// "no adapter" must not be confused with "reference cleared": both before and after
64+
// clearing, an unsupported type resolves to no adapter and fails the same way
65+
def err = shouldFail(IllegalArgumentException) {
66+
AwaitableAdapterRegistry.toAwaitable(new Object())
67+
}
68+
assert err.message.contains('No Awaitable adapter found')
69+
70+
cache().get(Object).clear()
71+
72+
err = shouldFail(IllegalArgumentException) {
73+
AwaitableAdapterRegistry.toAwaitable(new Object())
74+
}
75+
assert err.message.contains('No Awaitable adapter found')
76+
}
77+
78+
@Test
79+
void testRegistrationStillRebuildsTheCache() {
80+
def marker = new AwaitableAdapter() {
81+
@Override
82+
boolean supportsAwaitable(Class<?> type) { type == StringBuilder }
83+
84+
@Override
85+
def <T> Awaitable<T> toAwaitable(Object source) { Awaitable.of(source.toString()) }
86+
}
87+
try {
88+
AwaitableAdapterRegistry.register(marker)
89+
assert AwaitableAdapterRegistry.toAwaitable(new StringBuilder('sb')).get() == 'sb'
90+
} finally {
91+
AwaitableAdapterRegistry.unregister(marker)
92+
}
93+
shouldFail(IllegalArgumentException) {
94+
AwaitableAdapterRegistry.toAwaitable(new StringBuilder('sb'))
95+
}
96+
}
97+
}

0 commit comments

Comments
 (0)