Sitelet https://github.com/formancehq/numscript/pull/211
Skip to content

fix(vm): release the store when Exec returns so a reused VM does not retain it - #211

Merged
ascandone merged 1 commit into
mainfrom
fix/vm-release-store
Oct 6, 2026
Merged

ascandone merged 1 commit into
mainfrom
fix/vm-release-store

Conversation

@ascandone

Copy link
Copy Markdown
Contributor

Problem

vm.Exec binds the caller's store into the VM's RunState and only replaces it at the next execution. A VM that is kept and reused (the intended usage: the ledger caches one warm VM per script) therefore keeps its last run's store — and everything that store references — reachable after Exec returns.

In the ledger, that store reaches the apply scope of the Raft proposal that ran the script, including the whole preloaded coverage plan, so each cached VM pins an old proposal until the same script runs again or the entry is evicted.

Fix

Exec now defers runstate.Reset(nil): on every exit (success, execution error, or a panic raised by the store) the store is released along with the per-run balances. Results are unaffected — GetPostings already returns copies taken before the deferred reset.

Tests

  • TestExecVmReleasesStore holds a weak pointer to the store and checks it becomes collectable while the VM is still alive, for the success, error and store-panic exits. It fails on all three without the fix.
  • go test ./..., race on the touched packages, and golangci-lint are clean.

Performance

No measurable change (BenchmarkCompiledVM, BenchmarkCompiledVMCapped, BenchmarkCompiledVMFanIn, 5 runs each): identical allocations, timings within noise.

@NumaryBot NumaryBot added risk: medium NumaryBot classified this pull request as medium risk. bot-reviewed NumaryBot completed its review workflow for the current head. review-approved The NumaryBot review gate is satisfied for the current head. labels Oct 5, 2026

@NumaryBot NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The required automated review completed with no remaining findings.

@ascandone
ascandone merged commit e0b55a6 into main Oct 6, 2026
10 of 11 checks passed
@ascandone
ascandone deleted the fix/vm-release-store branch October 6, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-reviewed NumaryBot completed its review workflow for the current head. review-approved The NumaryBot review gate is satisfied for the current head. risk: medium NumaryBot classified this pull request as medium risk.

Development

Successfully merging this pull request may close these issues.

2 participants