Skip to content

Commit 8feb6e1

Browse files
andrrossjainankitk
andauthored
Wrap checked exceptions in painless.DefBootstrap (#19706)
A behavior change in [JDK-25][1] means that ClassValue.getFromHashMap will now wrap any checked exceptions with `java.lang.Error`. The fix is to wrap checked exceptions in `ClassValue.computeValue` with a specific RuntimeException wrapper, and then unwrap it later when handling. [1]: https://bugs.openjdk.org/browse/JDK-8351996 Signed-off-by: Andrew Ross <andrross@amazon.com> Signed-off-by: Ankit Jain <jainankitk@apache.org> Co-authored-by: Ankit Jain <jainankitk@apache.org>
1 parent 3b8f1bf commit 8feb6e1

File tree

4 files changed

+36
-6
lines changed

4 files changed

+36
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2222
- Onboarding new maven snapshots publishing to s3 ([#19619](https://github.com/opensearch-project/OpenSearch/pull/19619))
2323
- Remove MultiCollectorWrapper and use MultiCollector in Lucene instead ([#19595](https://github.com/opensearch-project/OpenSearch/pull/19595))
2424
- Change implementation for `percentiles` aggregation for latency improvement ([#19648](https://github.com/opensearch-project/OpenSearch/pull/19648))
25+
- Wrap checked exceptions in painless.DefBootstrap to support JDK-25 ([#19706](https://github.com/opensearch-project/OpenSearch/pull/19706))
2526
- Refactor the ThreadPoolStats.Stats class to use the Builder pattern instead of constructors ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
2627
- Refactor the IndexingStats.Stats class to use the Builder pattern instead of constructors ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
2728

modules/lang-painless/src/main/java/org/opensearch/painless/DefBootstrap.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,17 @@ private DefBootstrap() {} // no instance!
112112
*/
113113
public static final int OPERATOR_EXPLICIT_CAST = 1 << 2;
114114

115+
/**
116+
* ClassValue.getFromHashMap will wrap checked exceptions with Errors. To get
117+
* around that we wrap any checked exceptions with this RuntimeException and
118+
* unwrap them later.
119+
*/
120+
static final class WrappedCheckedException extends RuntimeException {
121+
private WrappedCheckedException(Exception cause) {
122+
super(cause);
123+
}
124+
}
125+
115126
/**
116127
* CallSite that implements the polymorphic inlining cache (PIC).
117128
*/
@@ -204,7 +215,10 @@ protected MethodHandle computeValue(Class<?> receiverType) {
204215
try {
205216
return lookup(flavor, name, receiverType).asType(type);
206217
} catch (Throwable t) {
207-
Def.rethrow(t);
218+
switch (t) {
219+
case Exception e -> throw new WrappedCheckedException(e);
220+
default -> Def.rethrow(t);
221+
}
208222
throw new AssertionError();
209223
}
210224
}

modules/lang-painless/src/main/java/org/opensearch/painless/PainlessScript.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,18 @@ public interface PainlessScript {
6565
/**
6666
* Adds stack trace and other useful information to exceptions thrown
6767
* from a Painless script.
68-
* @param t The throwable to build an exception around.
68+
* @param originalThrowable The throwable to build an exception around.
6969
* @return The generated ScriptException.
7070
*/
71-
default ScriptException convertToScriptException(Throwable t, Map<String, List<String>> extraMetadata) {
71+
default ScriptException convertToScriptException(Throwable originalThrowable, Map<String, List<String>> extraMetadata) {
72+
final Throwable unwrapped = switch (originalThrowable) {
73+
case DefBootstrap.WrappedCheckedException w -> w.getCause();
74+
default -> originalThrowable;
75+
};
7276
// create a script stack: this is just the script portion
7377
List<String> scriptStack = new ArrayList<>();
7478
ScriptException.Position pos = null;
75-
for (StackTraceElement element : t.getStackTrace()) {
79+
for (StackTraceElement element : unwrapped.getStackTrace()) {
7680
if (WriterConstants.CLASS_NAME.equals(element.getClassName())) {
7781
// found the script portion
7882
int originalOffset = element.getLineNumber();
@@ -106,7 +110,14 @@ default ScriptException convertToScriptException(Throwable t, Map<String, List<S
106110
scriptStack.add(element.toString());
107111
}
108112
}
109-
ScriptException scriptException = new ScriptException("runtime error", t, scriptStack, getName(), PainlessScriptEngine.NAME, pos);
113+
ScriptException scriptException = new ScriptException(
114+
"runtime error",
115+
unwrapped,
116+
scriptStack,
117+
getName(),
118+
PainlessScriptEngine.NAME,
119+
pos
120+
);
110121
for (Map.Entry<String, List<String>> entry : extraMetadata.entrySet()) {
111122
scriptException.addMetadata(entry.getKey(), entry.getValue());
112123
}

modules/lang-painless/src/test/java/org/opensearch/painless/DefBootstrapTests.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import java.util.Collections;
4747
import java.util.HashMap;
4848

49+
import static org.hamcrest.Matchers.instanceOf;
50+
4951
public class DefBootstrapTests extends OpenSearchTestCase {
5052
private final PainlessLookup painlessLookup = PainlessLookupBuilder.buildFromAllowlists(Allowlist.BASE_ALLOWLISTS);
5153

@@ -157,9 +159,11 @@ public void testMegamorphic() throws Throwable {
157159
map.put("a", "b");
158160
assertEquals(2, (int) handle.invokeExact((Object) map));
159161

160-
final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> {
162+
final DefBootstrap.WrappedCheckedException wrapped = expectThrows(DefBootstrap.WrappedCheckedException.class, () -> {
161163
Integer.toString((int) handle.invokeExact(new Object()));
162164
});
165+
assertThat(wrapped.getCause(), instanceOf(IllegalArgumentException.class));
166+
final IllegalArgumentException iae = (IllegalArgumentException) wrapped.getCause();
163167
assertEquals("dynamic method [java.lang.Object, size/0] not found", iae.getMessage());
164168
assertTrue("Does not fail inside ClassValue.computeValue()", Arrays.stream(iae.getStackTrace()).anyMatch(e -> {
165169
return e.getMethodName().equals("computeValue") && e.getClassName().startsWith("org.opensearch.painless.DefBootstrap$PIC$");

0 commit comments

Comments
 (0)