Merge pull request 'fix/scope-parent-cycle' (#92) from fix/scope-parent-cycle into master

Reviewed-on: #92
This commit is contained in:
Sergey Chernov 2025-12-11 03:09:45 +03:00
commit bce88ced43
9 changed files with 216 additions and 28 deletions

View File

@ -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.

View File

@ -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) {

View File

@ -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)
}

View File

@ -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() {
println
}
}
fun ll() { Whatever() }
fun callTest1() {
val l = ll()
l.something()
"ok"
}
assertEquals("ok", callTest1())
"""
).trimIndent()
)
}
}

View File

@ -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<Obj, Obj> = (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())
}
}

View File

@ -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) {

View File

@ -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) {

View File

@ -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) {

View File

@ -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) {