From d15dfb6087bff901760c926a781dbdca4e788d8a Mon Sep 17 00:00:00 2001 From: sergeych Date: Thu, 11 Dec 2025 00:50:46 +0100 Subject: [PATCH 1/2] core: prevent scope parent-chain cycles when reusing pooled frames - Scope.resetForReuse: fully detach before re-parenting (clear state, parent=null, new frameId), then validate with ensureNoCycle and assign parent/args/pos/thisObj - ScopePool.borrow (JVM/Android/JS/Native/Wasm): defensive fallback to fresh Scope allocation if resetForReuse detects a cycle - docs: add docs/fix-scope-parent-cycle.md describing the change and expectations - test: add ScopeCycleRegressionTest to ensure instance method call pattern does not crash and returns "ok" --- docs/fix-scope-parent-cycle.md | 34 +++++++++++++++++++ .../net/sergeych/lyng/ScopePoolAndroid.kt | 16 ++++++--- .../kotlin/net/sergeych/lyng/Scope.kt | 14 +++++--- .../kotlin/ScopeCycleRegressionTest.kt | 34 +++++++++++++++++++ .../kotlin/net/sergeych/lyng/ScopePoolJs.kt | 16 ++++++--- .../kotlin/net/sergeych/lyng/ScopePoolJvm.kt | 16 +++++++-- .../net/sergeych/lyng/ScopePoolNative.kt | 16 ++++++--- .../kotlin/net/sergeych/lyng/ScopePoolWasm.kt | 16 ++++++--- 8 files changed, 134 insertions(+), 28 deletions(-) create mode 100644 docs/fix-scope-parent-cycle.md create mode 100644 lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt diff --git a/docs/fix-scope-parent-cycle.md b/docs/fix-scope-parent-cycle.md new file mode 100644 index 0000000..11e3fdd --- /dev/null +++ b/docs/fix-scope-parent-cycle.md @@ -0,0 +1,34 @@ +## Fix: prevent cycles in scope parent chain during pooled frame reuse + +### What changed +- Scope.resetForReuse now fully detaches a reused scope from its previous chain/state before re-parenting: + - sets `parent = null` and regenerates `frameId` + - clears locals/slots/bindings caches + - only after that, validates the new parent with `ensureNoCycle` and assigns it +- ScopePool.borrow on all targets (JVM, Android, JS, Native, Wasm) now has a defensive fallback: + - if `resetForReuse` throws `IllegalStateException` indicating a parent-chain cycle, the pool allocates a fresh `Scope` instead of failing. + +### Why +In some nested call patterns (notably instance method calls where an instance is produced by another function and immediately used), the same pooled `Scope` object can be rebound into a chain that already (transitively) contains it. Reassigning `parent` in that case forms a structural cycle, which `ensureNoCycle` correctly detects and throws. This could surface as: + +``` +IllegalStateException: cycle detected in scope parent chain assignment + at net.sergeych.lyng.Scope.ensureNoCycle(...) + at net.sergeych.lyng.Scope.resetForReuse(...) + at net.sergeych.lyng.ScopePool.borrow(...) + ... during instance method invocation +``` + +The fix removes the failure mode by: +1) Detaching the reused frame from its prior chain/state before validating and assigning the new parent. +2) Falling back to a new frame allocation if a cycle is still detected (extremely rare and cheap vs. a crash). + +### Expected effects +- Eliminates sporadic `cycle detected in scope parent chain` crashes during instance method invocation. +- No change to public API or normal semantics. +- Pooling remains enabled by default; the fallback only triggers on the detected cycle edge case. +- Negligible performance impact: fresh allocation is used only when a cycle would have crashed the VM previously. + +### Notes +- The fix is platform-wide (all ScopePool actuals are covered). +- We recommend adding/keeping a regression test that exercises: a class with a method, a function returning an instance, and an exported function calling the instance method. The test should pass without exceptions. diff --git a/lynglib/src/androidMain/kotlin/net/sergeych/lyng/ScopePoolAndroid.kt b/lynglib/src/androidMain/kotlin/net/sergeych/lyng/ScopePoolAndroid.kt index fc97ff9..6f50103 100644 --- a/lynglib/src/androidMain/kotlin/net/sergeych/lyng/ScopePoolAndroid.kt +++ b/lynglib/src/androidMain/kotlin/net/sergeych/lyng/ScopePoolAndroid.kt @@ -39,12 +39,18 @@ actual object ScopePool { actual fun borrow(parent: Scope, args: Arguments, pos: Pos, thisObj: Obj): Scope { val pool = pool() val s = if (pool.isNotEmpty()) pool.removeLast() else Scope(parent, args, pos, thisObj) - if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { - s.resetForReuse(parent, args, pos, thisObj) - } else { - s.frameId = nextFrameId() + return try { + if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { + s.resetForReuse(parent, args, pos, thisObj) + } else { + s.frameId = nextFrameId() + } + s + } catch (e: IllegalStateException) { + if (e.message?.contains("cycle") == true && e.message?.contains("scope parent chain") == true) { + Scope(parent, args, pos, thisObj) + } else throw e } - return s } actual fun release(scope: Scope) { diff --git a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Scope.kt b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Scope.kt index 7bba7c0..77cf5f0 100644 --- a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Scope.kt +++ b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Scope.kt @@ -302,11 +302,9 @@ open class Scope( * Clears locals and slots, assigns new frameId, and sets parent/args/pos/thisObj. */ fun resetForReuse(parent: Scope?, args: Arguments, pos: Pos, thisObj: Obj) { - ensureNoCycle(parent) - this.parent = parent - this.args = args - this.pos = pos - this.thisObj = thisObj + // Fully detach from any previous chain/state first to avoid residual ancestry + // that could interact badly with the new parent and produce a cycle. + this.parent = null this.skipScopeCreation = false // fresh identity for PIC caches this.frameId = nextFrameId() @@ -315,6 +313,12 @@ open class Scope( slots.clear() nameToSlot.clear() localBindings.clear() + // Now safe to validate and re-parent + ensureNoCycle(parent) + this.parent = parent + this.args = args + this.pos = pos + this.thisObj = thisObj // Pre-size local slots for upcoming parameter assignment where possible reserveLocalCapacity(args.list.size + 4) } diff --git a/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt b/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt new file mode 100644 index 0000000..d25bef9 --- /dev/null +++ b/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt @@ -0,0 +1,34 @@ +/* + * Regression test for scope parent-chain cycle when reusing pooled frames + */ + +import kotlinx.coroutines.test.runTest +import net.sergeych.lyng.eval +import kotlin.test.Test + +class ScopeCycleRegressionTest { + @Test + fun instanceMethodCallDoesNotCycle() = runTest { + eval( + ( + """ + class Whatever { + fun something() { + trace("something") + } + } + + fun ll() { Whatever() } + + fun callTest1() { + val l = ll() + l.something() + "ok" + } + + assertEquals("ok", callTest1()) + """ + ).trimIndent() + ) + } +} diff --git a/lynglib/src/jsMain/kotlin/net/sergeych/lyng/ScopePoolJs.kt b/lynglib/src/jsMain/kotlin/net/sergeych/lyng/ScopePoolJs.kt index 21c9ff2..f30dd6a 100644 --- a/lynglib/src/jsMain/kotlin/net/sergeych/lyng/ScopePoolJs.kt +++ b/lynglib/src/jsMain/kotlin/net/sergeych/lyng/ScopePoolJs.kt @@ -29,12 +29,18 @@ actual object ScopePool { actual fun borrow(parent: Scope, args: Arguments, pos: Pos, thisObj: Obj): Scope { val s = if (pool.isNotEmpty()) pool.removeLast() else Scope(parent, args, pos, thisObj) - if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { - s.resetForReuse(parent, args, pos, thisObj) - } else { - s.frameId = nextFrameId() + return try { + if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { + s.resetForReuse(parent, args, pos, thisObj) + } else { + s.frameId = nextFrameId() + } + s + } catch (e: IllegalStateException) { + if (e.message?.contains("cycle") == true && e.message?.contains("scope parent chain") == true) { + Scope(parent, args, pos, thisObj) + } else throw e } - return s } actual fun release(scope: Scope) { diff --git a/lynglib/src/jvmMain/kotlin/net/sergeych/lyng/ScopePoolJvm.kt b/lynglib/src/jvmMain/kotlin/net/sergeych/lyng/ScopePoolJvm.kt index 8ad2479..214c82a 100644 --- a/lynglib/src/jvmMain/kotlin/net/sergeych/lyng/ScopePoolJvm.kt +++ b/lynglib/src/jvmMain/kotlin/net/sergeych/lyng/ScopePoolJvm.kt @@ -33,9 +33,19 @@ actual object ScopePool { actual fun borrow(parent: Scope, args: Arguments, pos: Pos, thisObj: Obj): Scope { val pool = threadLocalPool.get() val s = if (pool.isNotEmpty()) pool.removeLast() else Scope(parent, args, pos, thisObj) - // Always reset state on borrow to guarantee fresh-frame semantics - s.resetForReuse(parent, args, pos, thisObj) - return s + return try { + // Always reset state on borrow to guarantee fresh-frame semantics + s.resetForReuse(parent, args, pos, thisObj) + s + } catch (e: IllegalStateException) { + // Defensive fallback: if a cycle in scope parent chain is detected during reuse, + // discard pooled instance for this call frame and allocate a fresh scope instead. + if (e.message?.contains("cycle") == true && e.message?.contains("scope parent chain") == true) { + Scope(parent, args, pos, thisObj) + } else { + throw e + } + } } actual fun release(scope: Scope) { diff --git a/lynglib/src/nativeMain/kotlin/net/sergeych/lyng/ScopePoolNative.kt b/lynglib/src/nativeMain/kotlin/net/sergeych/lyng/ScopePoolNative.kt index 27308df..8ae449c 100644 --- a/lynglib/src/nativeMain/kotlin/net/sergeych/lyng/ScopePoolNative.kt +++ b/lynglib/src/nativeMain/kotlin/net/sergeych/lyng/ScopePoolNative.kt @@ -29,12 +29,18 @@ actual object ScopePool { actual fun borrow(parent: Scope, args: Arguments, pos: Pos, thisObj: Obj): Scope { val s = if (pool.isNotEmpty()) pool.removeLast() else Scope(parent, args, pos, thisObj) - if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { - s.resetForReuse(parent, args, pos, thisObj) - } else { - s.frameId = nextFrameId() + return try { + if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { + s.resetForReuse(parent, args, pos, thisObj) + } else { + s.frameId = nextFrameId() + } + s + } catch (e: IllegalStateException) { + if (e.message?.contains("cycle") == true && e.message?.contains("scope parent chain") == true) { + Scope(parent, args, pos, thisObj) + } else throw e } - return s } actual fun release(scope: Scope) { diff --git a/lynglib/src/wasmJsMain/kotlin/net/sergeych/lyng/ScopePoolWasm.kt b/lynglib/src/wasmJsMain/kotlin/net/sergeych/lyng/ScopePoolWasm.kt index bcd1500..cd5590a 100644 --- a/lynglib/src/wasmJsMain/kotlin/net/sergeych/lyng/ScopePoolWasm.kt +++ b/lynglib/src/wasmJsMain/kotlin/net/sergeych/lyng/ScopePoolWasm.kt @@ -29,12 +29,18 @@ actual object ScopePool { actual fun borrow(parent: Scope, args: Arguments, pos: Pos, thisObj: Obj): Scope { val s = if (pool.isNotEmpty()) pool.removeLast() else Scope(parent, args, pos, thisObj) - if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { - s.resetForReuse(parent, args, pos, thisObj) - } else { - s.frameId = nextFrameId() + return try { + if (s.parent !== parent || s.args !== args || s.pos !== pos || s.thisObj !== thisObj) { + s.resetForReuse(parent, args, pos, thisObj) + } else { + s.frameId = nextFrameId() + } + s + } catch (e: IllegalStateException) { + if (e.message?.contains("cycle") == true && e.message?.contains("scope parent chain") == true) { + Scope(parent, args, pos, thisObj) + } else throw e } - return s } actual fun release(scope: Scope) { -- 2.43.0 From fd473a32d830f98c03cba37917c671bc49915e77 Mon Sep 17 00:00:00 2001 From: sergeych Date: Thu, 11 Dec 2025 00:55:05 +0100 Subject: [PATCH 2/2] refine some tests --- .../kotlin/ScopeCycleRegressionTest.kt | 2 +- lynglib/src/commonTest/kotlin/ScriptTest.kt | 82 +++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt b/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt index d25bef9..4d30007 100644 --- a/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt +++ b/lynglib/src/commonTest/kotlin/ScopeCycleRegressionTest.kt @@ -14,7 +14,7 @@ class ScopeCycleRegressionTest { """ class Whatever { fun something() { - trace("something") + println } } diff --git a/lynglib/src/commonTest/kotlin/ScriptTest.kt b/lynglib/src/commonTest/kotlin/ScriptTest.kt index 8a37827..e6f36a8 100644 --- a/lynglib/src/commonTest/kotlin/ScriptTest.kt +++ b/lynglib/src/commonTest/kotlin/ScriptTest.kt @@ -4159,4 +4159,86 @@ class ScriptTest { """) } + @Test + fun testUsingClassConstructorVars() = runTest { + val r = eval(""" + import lyng.time + + class Request { + static val id = "rqid" + } + enum Action { + Test + } + class LogEntry(vaultId, action, data=null, createdAt=Instant.now().truncateToSecond()) { + + /* + Convert to a map object that can be easily decoded outsude the + contract execution scope. + */ + fun toApi() { + { createdAt:, requestId: Request.id, vaultId:, action: action.name, data: Map() } + } + } + fun test() { + LogEntry("v1", Action.Test).toApi() + } + + test() + """.trimIndent()).toJson() + println(r) + } + + @Test + fun testScopeShortCircuit() = runTest() { + val baseScope = Script.newScope() + + baseScope.eval(""" + val exports = Map() + fun Export(name,f) { + exports[name] = f + f + } + """.trimIndent() + ) + + val exports: MutableMap = (baseScope.eval("exports") as ObjMap).map + + baseScope.eval(""" + class A(val a) { + fun methodA() { + a + 1 + } + } + val a0 = 100 + + fun someFunction(x) { + val ia = A(x) + ia.methodA() + } + + @Export + fun exportedFunction(x) { + someFunction(x) + } + """.trimIndent()) + // Calling from the script is ok: + val instanceScope = baseScope.createChildScope() + instanceScope.eval(""" + val a1 = a0 + 1 + """.trimIndent()) + assertEquals( ObjInt(2), instanceScope.eval(""" + exportedFunction(1) + """)) + assertEquals( ObjInt(103), instanceScope.eval(""" + exportedFunction(a1 + 1) + """)) + val dummyThis = Obj() + // but we should be able to call it directly + val otherScope = baseScope.createChildScope() + val r = (exports["exportedFunction".toObj()] as Statement).invoke(otherScope, dummyThis,ObjInt(50)) + println(r) + assertEquals(51, r.toInt()) + } + } -- 2.43.0