Skip to content

Commit 7264867

Browse files
JaroslavTulachJaroslavTulach
authored
Transfer stack trace elements from the other JVM when rethrowing (#14666)
- @4e6 has troubles to obtain exceptions from _dual JVM mode_ - in his #14438 - this PR intends to make stacktraces of exceptions more useful
1 parent 4558a07 commit 7264867

File tree

6 files changed

+264
-40
lines changed

6 files changed

+264
-40
lines changed

‎lib/java/jvm-channel/src/main/java/org/enso/jvm/channel/Channel.java‎

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public static synchronized <D extends Config> Channel<D> create(
194194
arg.addressOf(2).setLong(CALLBACK_FN.getFunctionPointer().rawValue());
195195
arg.addressOf(3).setJObject(poolClassInHotSpot);
196196
var replyOk = fn.getCallStaticBooleanMethodA().call(e, channelClass, createMethod, arg);
197-
channel.checkForException(e, false);
197+
channel.checkUnexpectedException(e);
198198
assert replyOk : "Failed to create peer in HotSpot JVM";
199199

200200
ID_TO_CHANNEL.put(id, channel);
@@ -289,21 +289,24 @@ private static long acceptRequestFromHotSpotJvm(
289289

290290
var channel = ID_TO_CHANNEL.get(id);
291291
assert channel != null : "There must be a channel " + id + " but " + ID_TO_CHANNEL;
292+
var buf = asNativeByteBuffer(data, size);
293+
var len = handleWithChannel(channel, buf);
294+
return len;
295+
}
296+
297+
private static long handleWithChannel(Channel channel, ByteBuffer buf) {
292298
try {
293-
var buf = asNativeByteBuffer(data, size);
294-
var len = handleWithChannel(channel, buf);
295-
return len;
299+
return handleWithChannelThrow(channel, buf);
296300
} catch (Throwable ex) {
297-
channel.printStackTrace(ex, true);
298-
var bytes = ex.getMessage() == null ? new byte[0] : ex.getMessage().getBytes();
299-
var buf = asNativeByteBuffer(data, size);
300-
buf.putInt(bytes.length);
301-
buf.put(bytes);
301+
buf.position(0);
302+
ChannelExceptions.exceptionSerialize(buf, ex, Channel.class.getName(), STOP_METHOD_NAME);
302303
return RET_CODE_EXCEPTION;
303304
}
304305
}
305306

306-
private static long handleWithChannel(Channel channel, ByteBuffer buf) throws IOException {
307+
private static final String STOP_METHOD_NAME = "handleWithChannelThrow";
308+
309+
private static long handleWithChannelThrow(Channel channel, ByteBuffer buf) throws Throwable {
307310
// clean any previous overflow buffer
308311
keepLastOverflowBuffer.set(null);
309312

@@ -338,7 +341,7 @@ private long toHotSpotMessage(long address, long size) {
338341
arg.addressOf(2).setLong(address);
339342
arg.addressOf(3).setLong(size);
340343
var replySize = fn.getCallStaticLongMethodA().call(env, channelClass, channelHandle, arg);
341-
checkForException(env, true);
344+
checkUnexpectedException(env);
342345
return replySize;
343346
}
344347

@@ -363,7 +366,8 @@ private long toSubstrateMessage(MemorySegment seg) {
363366
return res;
364367
}
365368
} catch (Throwable ex) {
366-
printStackTrace(ex, false);
369+
// unexpected exception
370+
ex.printStackTrace();
367371
return -1L;
368372
}
369373
}
@@ -376,15 +380,13 @@ private long toDirectMessage(ByteBuffer buf) throws IOException {
376380
return len;
377381
}
378382

379-
private void checkForException(JNI.JNIEnv e, boolean userCode) {
383+
private void checkUnexpectedException(JNI.JNIEnv e) {
380384
var fn = e.getFunctions();
381385
var hasException = fn.getExceptionCheck().call(e);
382386
if (hasException) {
383387
var throwable = fn.getExceptionOccurred().call(e);
384388
assert throwable.isNonNull() : "There must be a throwable";
385-
if (printStackTrace(null, userCode)) {
386-
fn.getExceptionDescribe().call(e);
387-
}
389+
fn.getExceptionDescribe().call(e);
388390
fn.getExceptionClear().call(e);
389391
try (var throwableInC = CTypeConversion.toCString("java/lang/Throwable");
390392
var messageInC = CTypeConversion.toCString("getMessage");
@@ -430,11 +432,7 @@ private <R> R executeImpl(
430432
if (len == RET_CODE_EXCEPTION) {
431433
// signals exception
432434
buffer.position(0);
433-
var msgLen = buffer.getInt();
434-
var msgBytes = new byte[msgLen];
435-
buffer.get(msgBytes);
436-
var exceptionMessage = new String(msgBytes);
437-
throw new IllegalStateException(exceptionMessage);
435+
throw ChannelExceptions.exceptionDeserialize(RuntimeException.class, buffer);
438436
}
439437
if (len == RET_CODE_OVERFLOW) {
440438
buffer.position(0);
@@ -487,21 +485,6 @@ public void close() throws Exception {
487485
// TBD remove on the peer as well
488486
}
489487

490-
/**
491-
* @param ex exception to print stack trace for or {@code null}
492-
* @param userCode is the exception from user code or is it unexpected
493-
* @return {@code true} if the exception was printed and further details should be printed
494-
*/
495-
private boolean printStackTrace(Throwable ex, boolean userCode) {
496-
if (!userCode) {
497-
if (ex != null) {
498-
ex.printStackTrace();
499-
}
500-
return true;
501-
}
502-
return false;
503-
}
504-
505488
/**
506489
* Set of methods necessary for construction of a {@link Channel}. Subclasses must have a public
507490
* default constructor accessible via reflection from the {@link Channel#create} method.

‎lib/java/jvm-channel/src/main/java/org/enso/jvm/channel/ChannelExceptions.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package org.enso.jvm.channel;
2+
3+
import java.nio.ByteBuffer;
4+
import java.nio.charset.StandardCharsets;
5+
import java.util.ArrayList;
6+
import java.util.List;
7+
8+
final class ChannelExceptions {
9+
private static final int EOS = -1;
10+
private static final int NULL = -2;
11+
12+
private ChannelExceptions() {}
13+
14+
private static String readUTF(ByteBuffer buf) {
15+
int len = buf.getInt();
16+
assert len != EOS : "Unexpected end of stream in the buffer!";
17+
if (len == NULL) {
18+
return null;
19+
} else {
20+
byte[] bytes = new byte[len];
21+
buf.get(bytes);
22+
return new String(bytes, StandardCharsets.UTF_8);
23+
}
24+
}
25+
26+
private static boolean putUTF(ByteBuffer buf, String txt) {
27+
byte[] bytes = txt == null ? null : txt.getBytes(StandardCharsets.UTF_8);
28+
int wholeLen = bytes == null ? Integer.BYTES : bytes.length + Integer.BYTES;
29+
if (wholeLen > buf.remaining()) {
30+
return false;
31+
} else {
32+
if (bytes == null) {
33+
buf.putInt(NULL);
34+
} else {
35+
buf.putInt(bytes.length);
36+
buf.put(bytes);
37+
}
38+
return true;
39+
}
40+
}
41+
42+
static void exceptionSerialize(
43+
ByteBuffer buf, Throwable ex, String stopClass, String stopMethod) {
44+
boolean okClass = putUTF(buf, ex.getClass().getName());
45+
boolean okMsg = putUTF(buf, ex.getMessage());
46+
assert okClass && okMsg;
47+
int lastGood = 0;
48+
for (StackTraceElement elem : ex.getStackTrace()) {
49+
lastGood = buf.position();
50+
if (stopMethod.equals(elem.getMethodName()) && stopClass.equals(elem.getClassName())) {
51+
break;
52+
}
53+
boolean ok =
54+
putUTF(buf, elem.getClassName())
55+
&& putUTF(buf, elem.getMethodName())
56+
&& putUTF(buf, elem.getFileName());
57+
if (ok && buf.remaining() >= Integer.BYTES) {
58+
buf.putInt(elem.getLineNumber());
59+
if (buf.remaining() < Integer.BYTES) {
60+
break;
61+
}
62+
} else {
63+
buf.putInt(lastGood, EOS);
64+
return;
65+
}
66+
}
67+
buf.putInt(EOS);
68+
}
69+
70+
@SuppressWarnings(value = "unchecked")
71+
static <E extends Throwable> E exceptionDeserialize(Class<E> type, ByteBuffer buf) throws E {
72+
var clazz = readUTF(buf);
73+
var msg = readUTF(buf);
74+
var stack = new ArrayList<StackTraceElement>();
75+
while (true) {
76+
int peekLen = buf.getInt(buf.position());
77+
if (peekLen == EOS) {
78+
break;
79+
}
80+
String className = readUTF(buf);
81+
String methodName = readUTF(buf);
82+
String fileName = readUTF(buf);
83+
int lineNo = buf.getInt();
84+
StackTraceElement ste = new StackTraceElement(className, methodName, fileName, lineNo);
85+
stack.add(ste);
86+
}
87+
var ex =
88+
switch (clazz) {
89+
case "java.lang.NullPointerException" -> new NullPointerException(msg);
90+
case "java.lang.IllegalArgumentException" -> new IllegalArgumentException(msg);
91+
case "java.lang.IllegalStateException" -> new IllegalStateException(msg);
92+
case "java.lang.ClassCastException" -> new ClassCastException(msg);
93+
default -> new IllegalStateException(clazz + ":" + msg);
94+
};
95+
stack.addAll(List.of(ex.getStackTrace()));
96+
ex.setStackTrace(stack.toArray(StackTraceElement[]::new));
97+
throw (E) ex;
98+
}
99+
}

‎lib/java/jvm-channel/src/test/java/org/enso/jvm/channel/ChannelInSingleJvmTest.java‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
import static org.junit.Assert.assertEquals;
44
import static org.junit.Assert.assertFalse;
5+
import static org.junit.Assert.assertNotEquals;
56
import static org.junit.Assert.assertNotNull;
67
import static org.junit.Assert.assertTrue;
8+
import static org.junit.Assert.fail;
79

810
import java.util.function.Function;
911
import org.enso.persist.Persistable;
@@ -76,6 +78,86 @@ public void longText() {
7678
assertEquals(newMsg.text(), 32632, newMsg.text().length());
7779
}
7880

81+
@Test
82+
public void exceptionIsThrows() {
83+
var ch = Channel.create(null, PrivateData.class);
84+
85+
var msg = new GenerateString(-73);
86+
try {
87+
var newMsg = ch.execute(LongString.class, msg);
88+
fail("Not expecting a return value: " + newMsg);
89+
} catch (IllegalArgumentException ex) {
90+
assertEquals("Length must be positive. Was: -73", ex.getMessage());
91+
var stackTop = ex.getStackTrace()[0];
92+
assertEquals(GenerateString.class.getName(), stackTop.getClassName());
93+
assertEquals("handleGenerationOfStrings", stackTop.getMethodName());
94+
}
95+
}
96+
97+
@Test
98+
public void throwFactorialOne() throws Exception {
99+
assertException("1", new CountDownAndThrow(1, 1));
100+
}
101+
102+
@Test
103+
public void throwFactorialTwo() throws Exception {
104+
assertException("2", new CountDownAndThrow(2, 1));
105+
}
106+
107+
@Test
108+
public void throwFactorialThree() throws Exception {
109+
assertException("6", new CountDownAndThrow(3, 1));
110+
}
111+
112+
@Test
113+
public void throwFactorialFour() throws Exception {
114+
assertException("24", new CountDownAndThrow(4, 1));
115+
}
116+
117+
@Test
118+
public void throwFactorialFive() throws Exception {
119+
assertException("120", new CountDownAndThrow(5, 1));
120+
}
121+
122+
private void assertException(String msg, CountDownAndThrow action) {
123+
var channel = Channel.create(null, PrivateData.class);
124+
try {
125+
channel.execute(Void.class, action);
126+
fail("Expecting an exception to be thrown for " + msg);
127+
} catch (IllegalStateException ex) {
128+
assertEquals(msg, ex.getMessage());
129+
var countDecrementAndSendMessage = 0;
130+
for (var elem : ex.getStackTrace()) {
131+
if ("decrementAndSendMessage".equals(elem.getMethodName())) {
132+
assertEquals("ChannelInSingleJvmTest.java", elem.getFileName());
133+
assertNotEquals(-1, elem.getLineNumber());
134+
assertEquals(action.getClass().getName(), elem.getClassName());
135+
countDecrementAndSendMessage++;
136+
}
137+
}
138+
if (action.value() != countDecrementAndSendMessage) {
139+
ex.printStackTrace();
140+
assertEquals(
141+
"There is exactly right amount of invocations",
142+
action.value(),
143+
countDecrementAndSendMessage);
144+
}
145+
}
146+
}
147+
148+
@Test
149+
public void verifyStopMethodNameReferencesRealMethodName() throws Exception {
150+
var stopMethodField = Channel.class.getDeclaredField("STOP_METHOD_NAME");
151+
stopMethodField.setAccessible(true);
152+
var stopMethodValue = stopMethodField.get(null);
153+
for (var m : Channel.class.getDeclaredMethods()) {
154+
if (m.getName().equals(stopMethodValue)) {
155+
return;
156+
}
157+
}
158+
fail("STOP_METHOD_NAME field value should be consistent with method name");
159+
}
160+
79161
@Persistable(id = 8341)
80162
static final class Increment implements Function<Channel<?>, Increment> {
81163
int valueToIncrement;
@@ -111,7 +193,14 @@ static record GenerateString(int lengthToGenerate)
111193
implements Function<Channel<PrivateData>, LongString> {
112194
@Override
113195
public LongString apply(Channel<PrivateData> t) {
114-
return new LongString(lengthToGenerate);
196+
return handleGenerationOfStrings(lengthToGenerate);
197+
}
198+
199+
private static LongString handleGenerationOfStrings(int len) {
200+
if (len < 0) {
201+
throw new IllegalArgumentException("Length must be positive. Was: " + len);
202+
}
203+
return new LongString(len);
115204
}
116205
}
117206

@@ -121,4 +210,21 @@ private LongString(int len) {
121210
this("Hello".repeat(len / 5) + "!!!!!".substring(5 - len % 5));
122211
}
123212
}
213+
214+
@Persistable(id = 8345)
215+
record CountDownAndThrow(long value, long acc) implements Function<Channel<?>, Void> {
216+
@Override
217+
public Void apply(Channel<?> otherVM) {
218+
decrementAndSendMessage(value, acc, otherVM);
219+
return null;
220+
}
221+
222+
private static void decrementAndSendMessage(long n, long sum, Channel<?> otherVM) {
223+
if (n <= 1) {
224+
throw new IllegalStateException("" + sum);
225+
} else {
226+
otherVM.execute(Void.class, new CountDownAndThrow(n - 1, sum * n));
227+
}
228+
}
229+
}
124230
}

‎lib/java/jvm-interop/src/test/java/org/enso/jvm/interop/impl/OtherJvmObjectTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.math.BigDecimal;
2020
import java.nio.ByteBuffer;
2121
import java.time.Duration;
22+
import java.util.Arrays;
23+
import java.util.List;
2224
import java.util.function.Consumer;
2325
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
2426
import org.enso.jvm.channel.Channel;
@@ -137,6 +139,7 @@ public static short otherJvmValueOf(String txt) {
137139

138140
@Test
139141
public void parsingException() throws Exception {
142+
List<StackTraceElement> singleStack = null;
140143
var shortClass1 = ctx.asValue(java.lang.Short.class).getMember("static");
141144
try {
142145
var value1 = shortClass1.invokeMember("valueOf", "not-a-number");
@@ -145,17 +148,28 @@ public void parsingException() throws Exception {
145148
MatcherAssert.assertThat(e.getMessage(), CoreMatchers.containsString("not-a-number"));
146149
assertTrue("This is host exception", e.isHostException());
147150
assertNotNull("Host exception found", e.asHostException());
151+
singleStack = takeFew(5, e.getStackTrace());
148152
}
153+
assertNotNull("Stacktraces captured", singleStack);
149154
var shortClass2 = loadOtherJvmClass("java.lang.Short");
150155
try {
151156
var value2 = shortClass2.invokeMember("valueOf", "not-a-number");
152157
fail("Unexpected returned value: " + value2);
153158
} catch (PolyglotException e) {
154159
MatcherAssert.assertThat(e.getMessage(), CoreMatchers.containsString("not-a-number"));
155160
assertFalse("Alas this cannot be host exception", e.isHostException());
161+
var stack = e.getGuestObject().invokeMember("getStackTrace");
162+
for (var i = 0; i < 5; i++) {
163+
var elem = stack.getArrayElement(i);
164+
assertEquals(singleStack.get(i).toString(), elem.toString());
165+
}
156166
}
157167
}
158168

169+
private static List<StackTraceElement> takeFew(int n, StackTraceElement[] arr) {
170+
return Arrays.asList(arr).subList(0, n);
171+
}
172+
159173
@Test
160174
public void parsingWithGoodArguments() throws Exception {
161175
var longClass1 = ctx.asValue(java.lang.Long.class).getMember("static");

0 commit comments

Comments
 (0)