Skip to content

Commit c0bd605

Browse files
committed
Fix suspend Kotlin method instrumentation
suspend method in Kotlin generates a special bytecode for the method with a state machine and could return different objects. for each return branch we need to have a specific return handler to avoid having mismatch stack maps
1 parent d5d8f57 commit c0bd605

File tree

2 files changed

+20
-8
lines changed

2 files changed

+20
-8
lines changed

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/Instrumentor.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,15 @@ private AbstractInsnNode findFirstInsnForConstructor(AbstractInsnNode first) {
150150

151151
protected void processInstructions() {
152152
AbstractInsnNode node = methodNode.instructions.getFirst();
153-
while (node != null && !node.equals(returnHandlerLabel)) {
153+
LabelNode sentinelNode = new LabelNode();
154+
methodNode.instructions.add(sentinelNode);
155+
while (node != null && !node.equals(sentinelNode)) {
154156
if (node.getType() != AbstractInsnNode.LINE) {
155157
node = processInstruction(node);
156158
}
157159
node = node.getNext();
158160
}
161+
methodNode.instructions.remove(sentinelNode);
159162
if (returnHandlerLabel == null) {
160163
// if no return found, fallback to use the last instruction as last resort
161164
returnHandlerLabel = new LabelNode();
@@ -197,9 +200,8 @@ protected LabelNode getReturnHandler(AbstractInsnNode exitNode) {
197200
if (exitNode.getNext() != null || exitNode.getPrevious() != null) {
198201
throw new IllegalArgumentException("exitNode is not removed from original instruction list");
199202
}
200-
if (returnHandlerLabel != null) {
201-
return returnHandlerLabel;
202-
}
203+
// Create the returnHandlerLabel every time because the stack state could be different
204+
// for each return (suspend method in Kotlin)
203205
returnHandlerLabel = new LabelNode();
204206
methodNode.instructions.add(returnHandlerLabel);
205207
// stack top is return value (if any)

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -589,17 +589,27 @@ public void suspendKotlin() {
589589
public void suspendMethodKotlin() {
590590
final String CLASS_NAME = "CapturedSnapshot302";
591591
TestSnapshotListener listener =
592-
installProbes(createProbe(PROBE_ID, CLASS_NAME , "download", null));
592+
installProbes(createProbe(PROBE_ID, CLASS_NAME, "download", null));
593593
URL resource = CapturedSnapshotTest.class.getResource("/" + CLASS_NAME + ".kt");
594594
assertNotNull(resource);
595595
List<File> filesToDelete = new ArrayList<>();
596596
try {
597597
Class<?> testClass =
598598
KotlinHelper.compileAndLoad(CLASS_NAME, resource.getFile(), filesToDelete);
599599
Object companion = Reflect.onClass(testClass).get("Companion");
600-
int result = Reflect.on(companion).call("main", "").get();
601-
assertEquals(0, result);
602-
Snapshot snapshot = assertOneSnapshot(listener);
600+
int result = Reflect.on(companion).call("main", "1").get();
601+
assertEquals(1, result);
602+
// 2 snapshots are expected because the method is executed twice one for each state
603+
// before the delay, after the delay
604+
List<Snapshot> snapshots = assertSnapshots(listener, 2);
605+
Snapshot snapshot0 = snapshots.get(0);
606+
assertCaptureReturnValue(
607+
snapshot0.getCaptures().getReturn(),
608+
"kotlin.coroutines.intrinsics.CoroutineSingletons",
609+
"COROUTINE_SUSPENDED");
610+
Snapshot snapshot1 = snapshots.get(1);
611+
assertCaptureReturnValue(
612+
snapshot1.getCaptures().getReturn(), String.class.getTypeName(), "1");
603613
} finally {
604614
filesToDelete.forEach(File::delete);
605615
}

0 commit comments

Comments
 (0)