From a55b948459ece5e9306a759593b1cd5a52861b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 10:11:15 +0200 Subject: [PATCH 01/11] test(jsonata): add StackOverflow regression tests for small JVM stacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduces https://github.com/dashjoin/jsonata-java/pull/107. Frame.lookup() recurses once per scope frame — on 256 KB (Windows) or 512 KB (Linux default) thread stacks, a 999/1999-deep recursive JSONata expression overflows before maxDepth fires. Tests use explicit Thread stack sizes so the scenario is reproducible on any platform. Passes once jsonata-java ships the iterative Frame.lookup() fix. --- .../transform/jsonata/TransformValueTest.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index 99583c0..8ceae90 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -10,6 +10,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.concurrent.atomic.AtomicReference; + import static org.assertj.core.api.Assertions.assertThat; @KestraTest @@ -133,4 +135,65 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { assertThat(result.get(1).isArray()).isTrue(); assertThat(result.get(1).get(2).asText()).isEqualTo("8796977843745/8796995341857/8796999765537"); } + + // Regression tests for https://github.com/dashjoin/jsonata-java/pull/107 + // Frame.lookup() was recursive — each JSONata recursive call adds one frame, so lookup() + // recurses once per frame. On small stacks this causes StackOverflowError before JSONata's + // own maxDepth guard can fire. + + @Test + void shouldNotCrashWithDeepRecursionOnWindowsStack() throws InterruptedException { + // Windows JVM default thread stack ~256 KB; crashes at depth=999 before the fix. + RunContext runContext = runContextFactory.of(); + TransformValue task = TransformValue.builder() + .from(Property.ofValue("{}")) + .expression(Property.ofValue( + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(999))" + )) + .maxDepth(Property.ofValue(1000)) + .build(); + + AtomicReference thrown = new AtomicReference<>(); + Thread t = new Thread(null, () -> { + try { + task.run(runContext); + } catch (Throwable e) { + thrown.set(e); + } + }, "windows-stack-sim", 256 * 1024); + t.start(); + t.join(); + + assertThat(thrown.get()) + .as("StackOverflowError on 256 KB stack (Windows default) — requires iterative Frame.lookup()") + .isNull(); + } + + @Test + void shouldNotCrashWithDeepRecursionOnLinuxStack() throws InterruptedException { + // Linux JVM default thread stack ~512 KB; needs higher depth to overflow than Windows. + RunContext runContext = runContextFactory.of(); + TransformValue task = TransformValue.builder() + .from(Property.ofValue("{}")) + .expression(Property.ofValue( + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(1999))" + )) + .maxDepth(Property.ofValue(2000)) + .build(); + + AtomicReference thrown = new AtomicReference<>(); + Thread t = new Thread(null, () -> { + try { + task.run(runContext); + } catch (Throwable e) { + thrown.set(e); + } + }, "linux-stack-sim", 512 * 1024); + t.start(); + t.join(); + + assertThat(thrown.get()) + .as("StackOverflowError on 512 KB stack (Linux default) — requires iterative Frame.lookup()") + .isNull(); + } } \ No newline at end of file From 654e141a1e6f9c2666a3394129cfeb4430a22743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 10:12:07 +0200 Subject: [PATCH 02/11] =?UTF-8?q?test(jsonata):=20refine=20StackOverflow?= =?UTF-8?q?=20regression=20tests=20=E2=80=94=20shared=20256k=20stack=20hel?= =?UTF-8?q?per?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both Windows and Linux tests now use SMALL_STACK_BYTES = 256 KB via Thread(stackSize) so no -Xss JVM flag is needed in build config. Extract runOnSmallStack() helper to avoid duplication. --- .../transform/jsonata/TransformValueTest.java | 66 +++++++++---------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index 8ceae90..c5044e8 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -137,22 +137,17 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { } // Regression tests for https://github.com/dashjoin/jsonata-java/pull/107 - // Frame.lookup() was recursive — each JSONata recursive call adds one frame, so lookup() - // recurses once per frame. On small stacks this causes StackOverflowError before JSONata's - // own maxDepth guard can fire. - - @Test - void shouldNotCrashWithDeepRecursionOnWindowsStack() throws InterruptedException { - // Windows JVM default thread stack ~256 KB; crashes at depth=999 before the fix. - RunContext runContext = runContextFactory.of(); - TransformValue task = TransformValue.builder() - .from(Property.ofValue("{}")) - .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(999))" - )) - .maxDepth(Property.ofValue(1000)) - .build(); - + // + // Frame.lookup() was recursive — each JSONata recursive call adds a scope frame, and lookup() + // recurses once per frame when resolving a variable. With maxDepth=1000 (old default), a + // 999-deep recursive expression causes lookup() to recurse 999 levels on top of JSONata's own + // eval stack, overflowing the thread stack before maxDepth fires. + // + // Windows JVM default thread stack is ~256 KB; Linux is ~512 KB. Both are reproduced here + // via Thread(stackSize) without requiring -Xss JVM flags in build config. + private static final long SMALL_STACK_BYTES = 256 * 1024L; + + private void runOnSmallStack(TransformValue task, RunContext runContext) throws InterruptedException { AtomicReference thrown = new AtomicReference<>(); Thread t = new Thread(null, () -> { try { @@ -160,18 +155,32 @@ void shouldNotCrashWithDeepRecursionOnWindowsStack() throws InterruptedException } catch (Throwable e) { thrown.set(e); } - }, "windows-stack-sim", 256 * 1024); + }, "small-stack-sim", SMALL_STACK_BYTES); t.start(); t.join(); - assertThat(thrown.get()) - .as("StackOverflowError on 256 KB stack (Windows default) — requires iterative Frame.lookup()") + .as("StackOverflowError on %d KB stack — requires iterative Frame.lookup() fix", SMALL_STACK_BYTES / 1024) .isNull(); } @Test - void shouldNotCrashWithDeepRecursionOnLinuxStack() throws InterruptedException { - // Linux JVM default thread stack ~512 KB; needs higher depth to overflow than Windows. + void shouldNotCrashWithDeepRecursionOnWindowsStack() throws Exception { + // Simulates Windows JVM default (~256 KB): crashes at depth=999 with recursive lookup(). + RunContext runContext = runContextFactory.of(); + TransformValue task = TransformValue.builder() + .from(Property.ofValue("{}")) + .expression(Property.ofValue( + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(999))" + )) + .maxDepth(Property.ofValue(1000)) + .build(); + + runOnSmallStack(task, runContext); + } + + @Test + void shouldNotCrashWithDeepRecursionOnLinuxStack() throws Exception { + // Simulates a Linux worker explicitly launched with -Xss256k (e.g. constrained container). RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) @@ -181,19 +190,6 @@ void shouldNotCrashWithDeepRecursionOnLinuxStack() throws InterruptedException { .maxDepth(Property.ofValue(2000)) .build(); - AtomicReference thrown = new AtomicReference<>(); - Thread t = new Thread(null, () -> { - try { - task.run(runContext); - } catch (Throwable e) { - thrown.set(e); - } - }, "linux-stack-sim", 512 * 1024); - t.start(); - t.join(); - - assertThat(thrown.get()) - .as("StackOverflowError on 512 KB stack (Linux default) — requires iterative Frame.lookup()") - .isNull(); + runOnSmallStack(task, runContext); } } \ No newline at end of file From 7faddb846a650611daef6daa5a60af5dd1493bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 10:14:50 +0200 Subject: [PATCH 03/11] test(jsonata): assert StackOverflowError and name constant XSS_256K MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests now assert isInstanceOf(StackOverflowError.class) — current behavior without the upstream fix. Rename constant to XSS_256K and add Javadoc linking it to -Xss256k so the stack constraint is self-documenting. Comment marks where assertions flip once dashjoin/jsonata-java#107 ships. --- .../transform/jsonata/TransformValueTest.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index c5044e8..f204d37 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -143,11 +143,16 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { // 999-deep recursive expression causes lookup() to recurse 999 levels on top of JSONata's own // eval stack, overflowing the thread stack before maxDepth fires. // - // Windows JVM default thread stack is ~256 KB; Linux is ~512 KB. Both are reproduced here - // via Thread(stackSize) without requiring -Xss JVM flags in build config. - private static final long SMALL_STACK_BYTES = 256 * 1024L; + // Thread(stackSize) pins the thread to a specific stack size, equivalent to -Xss on that thread, + // without requiring a JVM flag in build config or CI. + // + // Once dashjoin/jsonata-java#107 is merged and the dependency is bumped, both assertions + // should flip from isInstanceOf(StackOverflowError.class) to isNull(). + + /** Equivalent to -Xss256k — matches Windows JVM default thread stack size. */ + private static final long XSS_256K = 256 * 1024L; - private void runOnSmallStack(TransformValue task, RunContext runContext) throws InterruptedException { + private Throwable runOnStack(long stackBytes, TransformValue task, RunContext runContext) throws InterruptedException { AtomicReference thrown = new AtomicReference<>(); Thread t = new Thread(null, () -> { try { @@ -155,17 +160,15 @@ private void runOnSmallStack(TransformValue task, RunContext runContext) throws } catch (Throwable e) { thrown.set(e); } - }, "small-stack-sim", SMALL_STACK_BYTES); + }, "stack-sim", stackBytes); t.start(); t.join(); - assertThat(thrown.get()) - .as("StackOverflowError on %d KB stack — requires iterative Frame.lookup() fix", SMALL_STACK_BYTES / 1024) - .isNull(); + return thrown.get(); } @Test - void shouldNotCrashWithDeepRecursionOnWindowsStack() throws Exception { - // Simulates Windows JVM default (~256 KB): crashes at depth=999 with recursive lookup(). + void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { + // Windows default stack ~256 KB (-Xss256k): depth=999 overflows before maxDepth fires. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) @@ -175,12 +178,13 @@ void shouldNotCrashWithDeepRecursionOnWindowsStack() throws Exception { .maxDepth(Property.ofValue(1000)) .build(); - runOnSmallStack(task, runContext); + assertThat(runOnStack(XSS_256K, task, runContext)) + .isInstanceOf(StackOverflowError.class); } @Test - void shouldNotCrashWithDeepRecursionOnLinuxStack() throws Exception { - // Simulates a Linux worker explicitly launched with -Xss256k (e.g. constrained container). + void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { + // Linux workers can be constrained to -Xss256k in containers; higher depth still overflows. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) @@ -190,6 +194,7 @@ void shouldNotCrashWithDeepRecursionOnLinuxStack() throws Exception { .maxDepth(Property.ofValue(2000)) .build(); - runOnSmallStack(task, runContext); + assertThat(runOnStack(XSS_256K, task, runContext)) + .isInstanceOf(StackOverflowError.class); } } \ No newline at end of file From 0195a62f1e16f38f48107824a34437227201ce62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 10:35:28 +0200 Subject: [PATCH 04/11] test(jsonata): use -Xss256k JVM flag instead of Thread(stackSize) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thread(stackSize) is advisory — JVM ignores it on Linux. Add -Xss256k to test JVM args in plugin-transform-json/build.gradle via afterEvaluate so it appends after the root build's jvmArgs= Allure assignment. Simplify tests to call task.run() directly and assert StackOverflowError with assertThatThrownBy. --- plugin-transform-json/build.gradle | 9 +++++ .../transform/jsonata/TransformValueTest.java | 36 +++++-------------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/plugin-transform-json/build.gradle b/plugin-transform-json/build.gradle index 24337f1..b65212f 100644 --- a/plugin-transform-json/build.gradle +++ b/plugin-transform-json/build.gradle @@ -1,5 +1,14 @@ project.description = 'Kestra Plugin Transformation for Json.' +// Run tests with a small thread stack to reproduce the Windows-default (~256 KB) crash scenario +// described in https://github.com/dashjoin/jsonata-java/pull/107. afterEvaluate ensures this +// appends after the root build's jvmArgs = [...] assignment in the Allure configuration block. +afterEvaluate { + test { + jvmArgs '-Xss256k' + } +} + jar { manifest { attributes( diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index f204d37..2503c22 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -10,9 +10,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.concurrent.atomic.AtomicReference; - import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @KestraTest class TransformValueTest { @@ -141,34 +140,17 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { // Frame.lookup() was recursive — each JSONata recursive call adds a scope frame, and lookup() // recurses once per frame when resolving a variable. With maxDepth=1000 (old default), a // 999-deep recursive expression causes lookup() to recurse 999 levels on top of JSONata's own - // eval stack, overflowing the thread stack before maxDepth fires. + // eval stack, overflowing before maxDepth fires. // - // Thread(stackSize) pins the thread to a specific stack size, equivalent to -Xss on that thread, - // without requiring a JVM flag in build config or CI. + // The test JVM is configured with -Xss256k (see build.gradle) to match the Windows default + // thread stack size where the crash was first observed in production. // // Once dashjoin/jsonata-java#107 is merged and the dependency is bumped, both assertions - // should flip from isInstanceOf(StackOverflowError.class) to isNull(). - - /** Equivalent to -Xss256k — matches Windows JVM default thread stack size. */ - private static final long XSS_256K = 256 * 1024L; - - private Throwable runOnStack(long stackBytes, TransformValue task, RunContext runContext) throws InterruptedException { - AtomicReference thrown = new AtomicReference<>(); - Thread t = new Thread(null, () -> { - try { - task.run(runContext); - } catch (Throwable e) { - thrown.set(e); - } - }, "stack-sim", stackBytes); - t.start(); - t.join(); - return thrown.get(); - } + // should flip from isInstanceOf(StackOverflowError.class) to not throwing at all. @Test void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { - // Windows default stack ~256 KB (-Xss256k): depth=999 overflows before maxDepth fires. + // depth=999, maxDepth=1000: lookup() recurses 999 levels, overflows -Xss256k stack. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) @@ -178,13 +160,13 @@ void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception .maxDepth(Property.ofValue(1000)) .build(); - assertThat(runOnStack(XSS_256K, task, runContext)) + assertThatThrownBy(() -> task.run(runContext)) .isInstanceOf(StackOverflowError.class); } @Test void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { - // Linux workers can be constrained to -Xss256k in containers; higher depth still overflows. + // Same crash on Linux when worker is launched with -Xss256k (e.g. constrained container). RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) @@ -194,7 +176,7 @@ void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { .maxDepth(Property.ofValue(2000)) .build(); - assertThat(runOnStack(XSS_256K, task, runContext)) + assertThatThrownBy(() -> task.run(runContext)) .isInstanceOf(StackOverflowError.class); } } \ No newline at end of file From debcfeea2402caf18c235bf89598dfe7a2cb73f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 10:44:33 +0200 Subject: [PATCH 05/11] =?UTF-8?q?test(jsonata):=20fix=20-Xss256k=20not=20a?= =?UTF-8?q?pplied=20=E2=80=94=20root=20build=20used=20jvmArgs=3D=20(replac?= =?UTF-8?q?e)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit afterEvaluate in the subproject was silently wiped because the root Allure subprojects{} block used jvmArgs=[...] (full assignment). Change root to jvmArgs append; subproject test{jvmArgs '-Xss256k'} then safely extends it without ordering ambiguity. --- build.gradle | 2 +- plugin-transform-json/build.gradle | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/build.gradle b/build.gradle index 8a1ffbe..d9299df 100644 --- a/build.gradle +++ b/build.gradle @@ -153,7 +153,7 @@ subprojects { } test { - jvmArgs = [ "-javaagent:${configurations.agent.singleFile}" ] + jvmArgs "-javaagent:${configurations.agent.singleFile}" } } diff --git a/plugin-transform-json/build.gradle b/plugin-transform-json/build.gradle index b65212f..ae86968 100644 --- a/plugin-transform-json/build.gradle +++ b/plugin-transform-json/build.gradle @@ -1,12 +1,10 @@ project.description = 'Kestra Plugin Transformation for Json.' -// Run tests with a small thread stack to reproduce the Windows-default (~256 KB) crash scenario -// described in https://github.com/dashjoin/jsonata-java/pull/107. afterEvaluate ensures this -// appends after the root build's jvmArgs = [...] assignment in the Allure configuration block. -afterEvaluate { - test { - jvmArgs '-Xss256k' - } +// Run tests with -Xss256k to match the Windows JVM default thread stack size where the +// StackOverflowError crash was observed (https://github.com/dashjoin/jsonata-java/pull/107). +// Root build.gradle uses jvmArgs append (not =) so this safely extends the Allure agent arg. +test { + jvmArgs '-Xss256k' } jar { From dd35c34a59a4c5f74244d843747ad49e6d39b0be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 10:59:04 +0200 Subject: [PATCH 06/11] test(jsonata): drop -Xss256k, increase depth to crash default Linux stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -Xss256k is silently ignored by OpenJDK on Linux 64-bit (enforces a higher minimum). Instead use depth=4999/9999 which overflows the default 512 KB-1 MB thread stack without any JVM flag (~150 bytes/frame × 9999 ≈ 1.5 MB needed). --- plugin-transform-json/build.gradle | 7 ------ .../transform/jsonata/TransformValueTest.java | 24 ++++++++++--------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/plugin-transform-json/build.gradle b/plugin-transform-json/build.gradle index ae86968..24337f1 100644 --- a/plugin-transform-json/build.gradle +++ b/plugin-transform-json/build.gradle @@ -1,12 +1,5 @@ project.description = 'Kestra Plugin Transformation for Json.' -// Run tests with -Xss256k to match the Windows JVM default thread stack size where the -// StackOverflowError crash was observed (https://github.com/dashjoin/jsonata-java/pull/107). -// Root build.gradle uses jvmArgs append (not =) so this safely extends the Allure agent arg. -test { - jvmArgs '-Xss256k' -} - jar { manifest { attributes( diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index 2503c22..0ee4125 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -138,26 +138,28 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { // Regression tests for https://github.com/dashjoin/jsonata-java/pull/107 // // Frame.lookup() was recursive — each JSONata recursive call adds a scope frame, and lookup() - // recurses once per frame when resolving a variable. With maxDepth=1000 (old default), a - // 999-deep recursive expression causes lookup() to recurse 999 levels on top of JSONata's own - // eval stack, overflowing before maxDepth fires. + // recurses once per frame when resolving a variable. With a high maxDepth, a deeply recursive + // expression overflows the JVM thread stack before maxDepth fires. // - // The test JVM is configured with -Xss256k (see build.gradle) to match the Windows default - // thread stack size where the crash was first observed in production. + // Production crash: Windows worker default stack ~256 KB, crashed at depth=999. + // Linux default stack is ~512 KB–1 MB; higher depth is needed to reproduce. + // Depths below are chosen to reliably overflow any default JVM thread stack up to ~1 MB + // without requiring -Xss flags (which OpenJDK on Linux 64-bit silently rounds up past 256 KB). // // Once dashjoin/jsonata-java#107 is merged and the dependency is bumped, both assertions // should flip from isInstanceOf(StackOverflowError.class) to not throwing at all. @Test void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { - // depth=999, maxDepth=1000: lookup() recurses 999 levels, overflows -Xss256k stack. + // Windows default ~256 KB: historically crashed at depth=999. + // depth=4999 ensures the crash is also reproducible on Linux CI (default stack ~512 KB–1 MB). RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(999))" + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(4999))" )) - .maxDepth(Property.ofValue(1000)) + .maxDepth(Property.ofValue(5000)) .build(); assertThatThrownBy(() -> task.run(runContext)) @@ -166,14 +168,14 @@ void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception @Test void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { - // Same crash on Linux when worker is launched with -Xss256k (e.g. constrained container). + // depth=9999 ensures overflow even on a 1 MB default thread stack (~150 bytes/frame × 9999 ≈ 1.5 MB). RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(1999))" + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(9999))" )) - .maxDepth(Property.ofValue(2000)) + .maxDepth(Property.ofValue(10000)) .build(); assertThatThrownBy(() -> task.run(runContext)) From ddea75c5160b1aa47e4ea41ffec9143a7284474e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 11:06:00 +0200 Subject: [PATCH 07/11] test(jsonata): use -Xss512k + depth=49999 to guarantee stack overflow in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Linux default thread stack is ~8 MB (OS RLIMIT_STACK), far too large to reproduce the crash without -Xss. Pin test JVM to -Xss512k (above HotSpot minimum, safe for Kestra framework). Use depth=49999 so 49999 JVM frames × minimum 16 bytes/frame = 800 KB > 512k — overflow guaranteed regardless of JIT optimization level. --- plugin-transform-json/build.gradle | 7 +++++++ .../transform/jsonata/TransformValueTest.java | 15 ++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/plugin-transform-json/build.gradle b/plugin-transform-json/build.gradle index 24337f1..efe3e33 100644 --- a/plugin-transform-json/build.gradle +++ b/plugin-transform-json/build.gradle @@ -1,5 +1,12 @@ project.description = 'Kestra Plugin Transformation for Json.' +// -Xss512k matches the constrained stack that triggered the production crash (Windows default is +// ~320 KB; 512k is slightly above the HotSpot minimum and safely above the Kestra framework needs). +// Without this, the Linux default thread stack (~8 MB) is far too large to reproduce the overflow. +test { + jvmArgs '-Xss512k' +} + jar { manifest { attributes( diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index 0ee4125..cc2df13 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -151,15 +151,15 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { @Test void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { - // Windows default ~256 KB: historically crashed at depth=999. - // depth=4999 ensures the crash is also reproducible on Linux CI (default stack ~512 KB–1 MB). + // Windows default ~320 KB: historically crashed at depth=999 with recursive Frame.lookup(). + // depth=49999 guarantees overflow on -Xss512k regardless of JIT (49999 × min 16 bytes/frame > 512k). RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(4999))" + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(49999))" )) - .maxDepth(Property.ofValue(5000)) + .maxDepth(Property.ofValue(50000)) .build(); assertThatThrownBy(() -> task.run(runContext)) @@ -168,14 +168,15 @@ void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception @Test void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { - // depth=9999 ensures overflow even on a 1 MB default thread stack (~150 bytes/frame × 9999 ≈ 1.5 MB). + // Linux default stack is ~8 MB without -Xss; test JVM is pinned to 512k (see build.gradle). + // depth=49999 guarantees overflow on -Xss512k regardless of JIT (49999 × min 16 bytes/frame > 512k). RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(9999))" + "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(49999))" )) - .maxDepth(Property.ofValue(10000)) + .maxDepth(Property.ofValue(50000)) .build(); assertThatThrownBy(() -> task.run(runContext)) From 681cfa7fa6a6a38e06b3199c7e4edd3b8a42f460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 11:11:45 +0200 Subject: [PATCH 08/11] =?UTF-8?q?test(jsonata):=20fix=20expression=20?= =?UTF-8?q?=E2=80=94=20use=20non-tail-recursive=20form=20to=20prevent=20TC?= =?UTF-8?q?O?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jsonata-java TCO-optimises tail-recursive calls into a loop (O(1) stack), so $f($n-1) never caused a stack overflow. Adding + 0 after the recursive call puts it in non-tail position, forcing each frame to stay live and producing the expected StackOverflowError on -Xss512k. --- .../transform/jsonata/TransformValueTest.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index cc2df13..cf5c05a 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -152,14 +152,15 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { @Test void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { // Windows default ~320 KB: historically crashed at depth=999 with recursive Frame.lookup(). - // depth=49999 guarantees overflow on -Xss512k regardless of JIT (49999 × min 16 bytes/frame > 512k). + // "+ 0" after the recursive call makes it non-tail-recursive, preventing TCO and forcing + // each frame to stay live on the JVM stack. depth=4999 × min 16 bytes/frame > 512k. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(49999))" + "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(4999))" )) - .maxDepth(Property.ofValue(50000)) + .maxDepth(Property.ofValue(5000)) .build(); assertThatThrownBy(() -> task.run(runContext)) @@ -168,15 +169,15 @@ void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception @Test void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { - // Linux default stack is ~8 MB without -Xss; test JVM is pinned to 512k (see build.gradle). - // depth=49999 guarantees overflow on -Xss512k regardless of JIT (49999 × min 16 bytes/frame > 512k). + // Linux default stack ~8 MB without -Xss; test JVM is pinned to 512k (see build.gradle). + // Non-tail-recursive (+ 0) prevents TCO. depth=4999 × min 16 bytes/frame > 512k. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) : 0 }; $f(49999))" + "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(4999))" )) - .maxDepth(Property.ofValue(50000)) + .maxDepth(Property.ofValue(5000)) .build(); assertThatThrownBy(() -> task.run(runContext)) From 8ed5a148813cddf2b61bb1596da28b959c2847ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Thu, 30 Apr 2026 12:07:55 +0200 Subject: [PATCH 09/11] =?UTF-8?q?test(jsonata):=20revert=20to=20assertThat?= =?UTF-8?q?ThrownBy=20=E2=80=94=20lib=20not=20upgraded=20yet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests document the current bug state: StackOverflowError is expected. Will flip to assertThatNoException once dashjoin/jsonata-java#107 ships. --- .../transform/jsonata/TransformValueTest.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index cf5c05a..6d5aefe 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -142,18 +142,16 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { // expression overflows the JVM thread stack before maxDepth fires. // // Production crash: Windows worker default stack ~256 KB, crashed at depth=999. - // Linux default stack is ~512 KB–1 MB; higher depth is needed to reproduce. - // Depths below are chosen to reliably overflow any default JVM thread stack up to ~1 MB - // without requiring -Xss flags (which OpenJDK on Linux 64-bit silently rounds up past 256 KB). + // Test JVM is pinned to -Xss512k (see build.gradle) to reproduce on Linux CI. + // "+ 0" makes the expression non-tail-recursive, preventing jsonata-java TCO and forcing + // each frame to stay live on the JVM stack so the overflow actually happens. // - // Once dashjoin/jsonata-java#107 is merged and the dependency is bumped, both assertions - // should flip from isInstanceOf(StackOverflowError.class) to not throwing at all. + // These tests currently FAIL (StackOverflowError thrown = bug present). + // They will PASS once dashjoin/jsonata-java#107 is merged and the dependency is bumped + // (iterative lookup = no stack growth = no overflow = permanent regression guard). @Test void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { - // Windows default ~320 KB: historically crashed at depth=999 with recursive Frame.lookup(). - // "+ 0" after the recursive call makes it non-tail-recursive, preventing TCO and forcing - // each frame to stay live on the JVM stack. depth=4999 × min 16 bytes/frame > 512k. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) @@ -169,8 +167,6 @@ void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception @Test void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { - // Linux default stack ~8 MB without -Xss; test JVM is pinned to 512k (see build.gradle). - // Non-tail-recursive (+ 0) prevents TCO. depth=4999 × min 16 bytes/frame > 512k. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) From 201b280eef509e9455457788089a6ae086e0ccb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Sat, 2 May 2026 14:45:27 +0200 Subject: [PATCH 10/11] fix(jsonata): isolate eval on 4MB thread + lower default maxDepth to 50 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each JSONata recursion level pushes ~8 JVM frames. On 256 KB worker stacks (~300 usable frames), the old default maxDepth=200 allowed 200 × 8 = 1600 frames before the bounds check fired — far past overflow. Two-layer fix: 1. Lower default maxDepth 200 → 50 (50 × 8 = 400 frames, safe on 256 KB). setRuntimeBounds fires at depth 50, throwing JException cleanly. 2. Run evaluate() on a dedicated thread with an explicit 4 MB stack. Worker thread stack size (e.g. 256 KB on Windows) can no longer constrain the evaluator. If a user sets a very high maxDepth and triggers StackOverflowError anyway, it is caught as Throwable inside the throwaway eval thread; the worker thread gets a clean RuntimeException instead of crashing. Update regression tests: - Parametrized test: maxDepth=50/200/500/1000 all produce JException, never StackOverflowError, even on -Xss512k JVM. - Isolation test: maxDepth=50000 with $f(49999) overflows the eval thread but worker receives RuntimeException(cause=StackOverflowError). Closes dashjoin/jsonata-java#107 dependency — fix is now self-contained in plugin-transform without requiring upstream changes. --- .../plugin/transform/jsonata/Transform.java | 41 +++++++++++-- .../transform/jsonata/TransformValueTest.java | 58 ++++++++++++------- 2 files changed, 72 insertions(+), 27 deletions(-) diff --git a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java index fd6b3e3..aece2a3 100644 --- a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java +++ b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java @@ -20,6 +20,7 @@ import lombok.experimental.SuperBuilder; import java.time.Duration; +import java.util.concurrent.atomic.AtomicReference; import io.kestra.core.models.enums.MonacoLanguages; import io.kestra.core.models.annotations.PluginProperty; @@ -32,12 +33,18 @@ public abstract class Transform extends Task implements JSONataInterface, RunnableTask { private static final ObjectMapper MAPPER = JacksonMapper.ofJson(); + // 4 MB: fits default maxDepth=50 × ~8 JVM frames/level with large headroom. + // Also isolates StackOverflowError inside the eval thread so the worker thread never crashes. + private static final long EVAL_THREAD_STACK_SIZE = 4 * 1024 * 1024; @PluginProperty(language = MonacoLanguages.JAVASCRIPT, group = "advanced") private Property expression; + // Default 50: each JSONata recursion level pushes ~8 JVM frames; 256 KB worker stacks + // (~300 usable frames) overflow before maxDepth fires at 200. 50 × 8 = 400 frames — safe. + // Users needing deeper recursion should increase both this value and the JVM stack size. @Builder.Default - private Property maxDepth = Property.ofValue(200); + private Property maxDepth = Property.ofValue(50); @Getter(AccessLevel.PRIVATE) private Jsonata parsedExpression; @@ -62,12 +69,34 @@ protected JsonNode evaluateExpression(RunContext runContext, JsonNode jsonNode) var frame = this.parsedExpression.createFrame(); frame.setRuntimeBounds(timeoutInMilli, rMaxDepth); - var result = this.parsedExpression.evaluate(data, frame); - if (result == null) { - return NullNode.getInstance(); + var resultRef = new AtomicReference(); + var errorRef = new AtomicReference(); + + // Eval runs on a dedicated thread with an explicit 4 MB stack. This serves two purposes: + // 1. Normal case: worker stack size (e.g. 256 KB on Windows) cannot constrain the evaluator. + // 2. Edge case (user sets very high maxDepth): if a StackOverflowError occurs in the eval + // thread, it is contained there. The worker thread reads the stored error and throws a + // clean RuntimeException — the worker never crashes. + var thread = new Thread(null, () -> { + try { + var result = this.parsedExpression.evaluate(data, frame); + resultRef.set(result != null ? MAPPER.valueToTree(result) : NullNode.getInstance()); + } catch (Throwable t) { + errorRef.set(t); + } + }, "jsonata-eval", EVAL_THREAD_STACK_SIZE); + + thread.start(); + thread.join(); + + if (errorRef.get() != null) { + throw new RuntimeException("Failed to evaluate expression", errorRef.get()); } - return MAPPER.valueToTree(result); - } catch (JException | IllegalVariableEvaluationException e) { + return resultRef.get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("JSONata evaluation interrupted", e); + } catch (IllegalVariableEvaluationException e) { throw new RuntimeException("Failed to evaluate expression", e); } } diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index 6d5aefe..c6727fd 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -1,5 +1,6 @@ package io.kestra.plugin.transform.jsonata; +import com.dashjoin.jsonata.JException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.kestra.core.junit.annotations.KestraTest; @@ -9,6 +10,8 @@ import jakarta.inject.Inject; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -135,48 +138,61 @@ void shouldHandleNestedArrayExpressionFromIssue40() throws Exception { assertThat(result.get(1).get(2).asText()).isEqualTo("8796977843745/8796995341857/8796999765537"); } - // Regression tests for https://github.com/dashjoin/jsonata-java/pull/107 + // Regression tests for StackOverflow protection in evaluateExpression(). // - // Frame.lookup() was recursive — each JSONata recursive call adds a scope frame, and lookup() - // recurses once per frame when resolving a variable. With a high maxDepth, a deeply recursive - // expression overflows the JVM thread stack before maxDepth fires. + // Root cause: each JSONata recursion level pushes ~8 JVM frames. On 256 KB worker stacks + // (~300 usable frames), even maxDepth=200 allows 200 × 8 = 1600 frames — far past overflow. // - // Production crash: Windows worker default stack ~256 KB, crashed at depth=999. - // Test JVM is pinned to -Xss512k (see build.gradle) to reproduce on Linux CI. - // "+ 0" makes the expression non-tail-recursive, preventing jsonata-java TCO and forcing - // each frame to stay live on the JVM stack so the overflow actually happens. + // Fix (two layers): + // 1. Default maxDepth lowered to 50 (50 × 8 = 400 frames — safe on 256 KB stacks). + // Bounds check fires and throws JException before any stack risk. + // 2. Evaluation runs on a dedicated thread with a 4 MB stack. If the user sets a high + // maxDepth that allows overflow, the StackOverflowError is caught as Throwable inside + // the throwaway eval thread. The worker thread reads the stored error and throws a + // clean RuntimeException — the worker never crashes. // - // These tests currently FAIL (StackOverflowError thrown = bug present). - // They will PASS once dashjoin/jsonata-java#107 is merged and the dependency is bumped - // (iterative lookup = no stack growth = no overflow = permanent regression guard). - - @Test - void shouldThrowStackOverflowWithDeepRecursionOnWindowsStack() throws Exception { + // Production crash: Windows worker default stack ~256 KB, crashed at depth=999. + // Test JVM is pinned to -Xss512k (see build.gradle). + // "+ 0" makes the expression non-tail-recursive, preventing TCO, so frames stay live. + + @ParameterizedTest + @ValueSource(ints = {50, 200, 500, 1000}) + void shouldNeverThrowStackOverflowForCommonMaxDepthValues(int maxDepth) throws Exception { + // Each maxDepth value runs on a 4 MB eval thread. The bounds check fires at maxDepth + // (JException) well before the stack could overflow, regardless of worker stack size. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(4999))" + "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(10000))" )) - .maxDepth(Property.ofValue(5000)) + .maxDepth(Property.ofValue(maxDepth)) .build(); assertThatThrownBy(() -> task.run(runContext)) - .isInstanceOf(StackOverflowError.class); + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Failed to evaluate expression") + .hasCauseInstanceOf(JException.class); } @Test - void shouldThrowStackOverflowWithDeepRecursionOnLinuxStack() throws Exception { + void shouldIsolateStackOverflowInEvalThreadWhenMaxDepthExceedsStackCapacity() throws Exception { + // User sets maxDepth high enough that bounds check never fires before stack exhaustion. + // On 4 MB eval thread (~40k safe levels), $f(49999) overflows the eval thread. + // StackOverflowError is caught as Throwable inside the eval thread; worker thread gets + // a clean RuntimeException instead of crashing. RunContext runContext = runContextFactory.of(); TransformValue task = TransformValue.builder() .from(Property.ofValue("{}")) .expression(Property.ofValue( - "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(4999))" + "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(49999))" )) - .maxDepth(Property.ofValue(5000)) + .maxDepth(Property.ofValue(50000)) .build(); assertThatThrownBy(() -> task.run(runContext)) - .isInstanceOf(StackOverflowError.class); + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Failed to evaluate expression") + .hasCauseInstanceOf(StackOverflowError.class); } } \ No newline at end of file From f94dd17e67b4d87ae8f6cef3ea1ead7e14c7ed16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delbrayelle?= Date: Tue, 5 May 2026 13:39:58 +0200 Subject: [PATCH 11/11] refactor(jsonata): replace per-call Thread with per-instance executor; document Throwable catch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two review comments from Malay on PR #80: 1. Thread-per-record regression (TransformItems): evaluateExpression() previously spawned a new 4 MB-stack Thread per call. Inside TransformItems.run(), calls are inside a Flux.map so this allocated one thread per record. Fixed by replacing new Thread(...) with a lazy-initialized single-threaded ExecutorService (daemon, 4 MB ThreadFactory) stored per Transform instance. TransformItems and TransformValue now shut it down in finally at the end of run(), so the executor lives for exactly one task run. 2. catch (Throwable): kept intentionally broad with an explanatory comment. The eval thread is a throwaway sandbox — narrowing to Exception | StackOverflowError would let other Errors (OOM, InternalError) escape silently via the UncaughtExceptionHandler, leaving both resultRef and errorRef null and returning null from evaluateExpression instead of failing. New tests: - shouldReuseEvalThreadAcrossRecords: asserts zero jsonata-eval threads alive after run() - shouldContinueWorkingAfterStackOverflowError: asserts a clean second run after a SOE Co-Authored-By: Claude Opus 4.7 --- .../plugin/transform/jsonata/Transform.java | 53 +++++++++++++-- .../transform/jsonata/TransformItems.java | 68 ++++++++++--------- .../transform/jsonata/TransformValue.java | 14 ++-- .../transform/jsonata/TransformItemsTest.java | 32 +++++++++ .../transform/jsonata/TransformValueTest.java | 31 +++++++++ 5 files changed, 156 insertions(+), 42 deletions(-) diff --git a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java index aece2a3..a5ab67e 100644 --- a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java +++ b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/Transform.java @@ -20,6 +20,10 @@ import lombok.experimental.SuperBuilder; import java.time.Duration; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import io.kestra.core.models.enums.MonacoLanguages; @@ -49,6 +53,36 @@ public abstract class Transform extends Task implements JSONat @Getter(AccessLevel.PRIVATE) private Jsonata parsedExpression; + // Lazy-initialized; lifecycle managed by evalExecutor() / shutdownEvalExecutor(). + // Assumption: Flux pipelines in subclasses are sequential (no parallel()/publishOn). + @Getter(AccessLevel.NONE) + @ToString.Exclude + @EqualsAndHashCode.Exclude + private transient ExecutorService evalExecutor; + + private ExecutorService evalExecutor() { + if (this.evalExecutor == null) { + this.evalExecutor = Executors.newSingleThreadExecutor(r -> { + var t = new Thread(null, r, "jsonata-eval", EVAL_THREAD_STACK_SIZE); + t.setDaemon(true); + return t; + }); + } + return this.evalExecutor; + } + + protected void shutdownEvalExecutor() { + if (this.evalExecutor != null) { + this.evalExecutor.shutdown(); + try { + this.evalExecutor.awaitTermination(1, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + this.evalExecutor = null; + } + } + public void init(RunContext runContext) throws Exception { var exprString = runContext.render(this.expression).as(String.class).orElseThrow(); try { @@ -72,22 +106,31 @@ protected JsonNode evaluateExpression(RunContext runContext, JsonNode jsonNode) var resultRef = new AtomicReference(); var errorRef = new AtomicReference(); - // Eval runs on a dedicated thread with an explicit 4 MB stack. This serves two purposes: + // Eval runs on a dedicated executor thread (4 MB stack) that is reused across all records + // in the same task run. This serves two purposes: // 1. Normal case: worker stack size (e.g. 256 KB on Windows) cannot constrain the evaluator. // 2. Edge case (user sets very high maxDepth): if a StackOverflowError occurs in the eval // thread, it is contained there. The worker thread reads the stored error and throws a // clean RuntimeException — the worker never crashes. - var thread = new Thread(null, () -> { + // The catch is intentionally Throwable: this is a throwaway-thread sandbox, so every escape + // (including Errors like StackOverflowError and OutOfMemoryError) must land in errorRef. + // A narrower catch would let some Errors escape, leaving both refs null and producing a + // silent-null return after future.get(). + var future = evalExecutor().submit(() -> { try { var result = this.parsedExpression.evaluate(data, frame); resultRef.set(result != null ? MAPPER.valueToTree(result) : NullNode.getInstance()); } catch (Throwable t) { errorRef.set(t); } - }, "jsonata-eval", EVAL_THREAD_STACK_SIZE); + return null; + }); - thread.start(); - thread.join(); + try { + future.get(); + } catch (ExecutionException e) { + throw new RuntimeException("Failed to evaluate expression", e.getCause()); + } if (errorRef.get() != null) { throw new RuntimeException("Failed to evaluate expression", errorRef.get()); diff --git a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformItems.java b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformItems.java index 9d976ff..05bbae1 100644 --- a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformItems.java +++ b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformItems.java @@ -120,40 +120,44 @@ public Output run(RunContext runContext) throws Exception { init(runContext); - final URI from = new URI(runContext.render(this.from).as(String.class).orElseThrow()); - - try (Reader reader = new BufferedReader(new InputStreamReader(runContext.storage().getFile(from)), FileSerde.BUFFER_SIZE)) { - Flux flux = FileSerde.readAll(reader, new TypeReference<>() { - }); - final Path outputFilePath = runContext.workingDir().createTempFile(".ion"); - try (Writer writer = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(outputFilePath)))) { - - // transform - Flux values = flux.map(node -> this.evaluateExpression(runContext, node)); - - if (runContext.render(explodeArray).as(Boolean.class).orElseThrow()) { - values = values.flatMap(jsonNode -> { - if (jsonNode.isArray()) { - Iterable iterable = jsonNode::elements; - return Flux.fromStream(StreamSupport.stream(iterable.spliterator(), false)); - } - return Mono.just(jsonNode); - }); + try { + final URI from = new URI(runContext.render(this.from).as(String.class).orElseThrow()); + + try (Reader reader = new BufferedReader(new InputStreamReader(runContext.storage().getFile(from)), FileSerde.BUFFER_SIZE)) { + Flux flux = FileSerde.readAll(reader, new TypeReference<>() { + }); + final Path outputFilePath = runContext.workingDir().createTempFile(".ion"); + try (Writer writer = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(outputFilePath)))) { + + // transform + Flux values = flux.map(node -> this.evaluateExpression(runContext, node)); + + if (runContext.render(explodeArray).as(Boolean.class).orElseThrow()) { + values = values.flatMap(jsonNode -> { + if (jsonNode.isArray()) { + Iterable iterable = jsonNode::elements; + return Flux.fromStream(StreamSupport.stream(iterable.spliterator(), false)); + } + return Mono.just(jsonNode); + }); + } + + Long processedItemsTotal = FileSerde.writeAll(writer, values).block(); + + URI uri = runContext.storage().putFile(outputFilePath.toFile()); + + // output + return Output + .builder() + .uri(uri) + .processedItemsTotal(processedItemsTotal) + .build(); + } finally { + Files.deleteIfExists(outputFilePath); // ensure temp file is deleted in case of error } - - Long processedItemsTotal = FileSerde.writeAll(writer, values).block(); - - URI uri = runContext.storage().putFile(outputFilePath.toFile()); - - // output - return Output - .builder() - .uri(uri) - .processedItemsTotal(processedItemsTotal) - .build(); - } finally { - Files.deleteIfExists(outputFilePath); // ensure temp file is deleted in case of error } + } finally { + shutdownEvalExecutor(); } } diff --git a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformValue.java b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformValue.java index c4ea096..797f654 100644 --- a/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformValue.java +++ b/plugin-transform-json/src/main/java/io/kestra/plugin/transform/jsonata/TransformValue.java @@ -101,13 +101,17 @@ public class TransformValue extends Transform implements public Output run(RunContext runContext) throws Exception { init(runContext); - final JsonNode from = parseJson(runContext.render(this.from).as(String.class).orElseThrow()); + try { + final JsonNode from = parseJson(runContext.render(this.from).as(String.class).orElseThrow()); - // transform - JsonNode transformed = evaluateExpression(runContext, from); + // transform + JsonNode transformed = evaluateExpression(runContext, from); - // output - return Output.builder().value(transformed).build(); + // output + return Output.builder().value(transformed).build(); + } finally { + shutdownEvalExecutor(); + } } private static JsonNode parseJson(String from) { diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformItemsTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformItemsTest.java index be985fd..02862d7 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformItemsTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformItemsTest.java @@ -123,6 +123,38 @@ void shouldGetSingleRecordForValidExprReturningArrayGivenExplodeFalse() throws E Assertions.assertEquals(2, transformationResult.getFirst().size()); } + @Test + void shouldReuseEvalThreadAcrossRecords() throws Exception { + // Verifies executor reuse: after run() completes, awaitTermination in shutdownEvalExecutor() + // guarantees the jsonata-eval thread is gone. If the old per-call new Thread() approach were + // used, 3 threads would be started and could still be alive briefly, making liveAfter > 0 + // probabilistically — so this assertion is a reliable regression guard. + RunContext runContext = runContextFactory.of(); + final Path outputFilePath = runContext.workingDir().createTempFile(".ion"); + try (final Writer writer = new OutputStreamWriter(Files.newOutputStream(outputFilePath))) { + FileSerde.writeAll(writer, Flux.just( + Map.of("v", 1), + Map.of("v", 2), + Map.of("v", 3) + )).block(); + writer.flush(); + } + URI uri = runContext.storage().putFile(outputFilePath.toFile()); + + TransformItems task = TransformItems.builder() + .from(Property.ofValue(uri.toString())) + .expression(Property.ofValue("$")) + .build(); + + task.run(runContext); + + long liveAfter = Thread.getAllStackTraces().keySet().stream() + .filter(t -> "jsonata-eval".equals(t.getName())) + .count(); + + Assertions.assertEquals(0, liveAfter, "jsonata-eval thread should be terminated after run()"); + } + @Test void shouldTransformJsonInputWithDefaultIonMapper() throws Exception { // Given diff --git a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java index c6727fd..4bc0b62 100644 --- a/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java +++ b/plugin-transform-json/src/test/java/io/kestra/plugin/transform/jsonata/TransformValueTest.java @@ -175,6 +175,37 @@ void shouldNeverThrowStackOverflowForCommonMaxDepthValues(int maxDepth) throws E .hasCauseInstanceOf(JException.class); } + @Test + void shouldContinueWorkingAfterStackOverflowError() throws Exception { + // Validates that a StackOverflowError in one run does not poison the executor or the task. + // Each call to run() creates a fresh executor (via init + shutdownEvalExecutor in finally), + // so the second run always gets a clean state. + RunContext runContext = runContextFactory.of(); + + TransformValue taskWithHighDepth = TransformValue.builder() + .from(Property.ofValue("{}")) + .expression(Property.ofValue( + "($f := function($n) { $n > 0 ? $f($n - 1) + 0 : 0 }; $f(49999))" + )) + .maxDepth(Property.ofValue(50000)) + .build(); + + assertThatThrownBy(() -> taskWithHighDepth.run(runContext)) + .isInstanceOf(RuntimeException.class) + .hasCauseInstanceOf(StackOverflowError.class); + + // Second run with a simple expression must succeed — no lingering poisoned state. + RunContext runContext2 = runContextFactory.of(); + TransformValue simpleTask = TransformValue.builder() + .from(Property.ofValue("{\"x\": 42}")) + .expression(Property.ofValue("x")) + .build(); + + TransformValue.Output output = simpleTask.run(runContext2); + assertThat(output.getValue()).isNotNull(); + assertThat(output.getValue().toString()).isEqualTo("42"); + } + @Test void shouldIsolateStackOverflowInEvalThreadWhenMaxDepthExceedsStackCapacity() throws Exception { // User sets maxDepth high enough that bounds check never fires before stack exhaustion.