[fix] apply the Machine step budget and print func to load()ed modules#171
Conversation
A module executed by load() ran in a bare starlark.Thread that inherited none of the Machine's execution context, so: - the step budget did not apply — a top-level loop in a load()ed source module ran unbounded, escaping the DoS guard (the budget only armed the main thread); - print() in a loaded module went to stdout via a hardcoded fmt.Println, bypassing the Machine's print func. The load cache now builds its child thread from the Machine (newLoadThread): the same print func, an independent copy of the step budget (with the MaxStepsExceeded panic), and the current run's context local. The budget is per-thread — enough to stop one runaway loaded module, which is the DoS the bare thread let through; it is not a shared aggregate counter. Because a loaded module can now panic (OnMaxSteps), the load cache's get() recovers around doLoad: it readies the entry with the error, drops it so a retry re-executes, and re-raises so runInternal maps it to a typed error. Without this the half-built cache entry was never readied, and a second Run of a module that hit the budget blocked forever on its ready channel. Not covered here (separate follow-ups, noted honestly rather than half-done): lazy loaders built with MakeModuleLoaderFromString/Reader/File still run on their own bare thread and inherit no budget/print (fixing needs an API change); and a context timeout cancels only the main thread, so a loaded module's pure-Starlark loop is bounded by the step budget, not the deadline. Test: TestMachine_LoadInheritsMachineContext — a loaded module hits the step budget, its print() reaches the Machine print func, and a second run after a budget panic returns promptly instead of deadlocking on the load cache. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 40 |
| Duplication | 2 |
🟢 Coverage 96.55% diff coverage · +0.09% coverage variation
Metric Results Coverage variation ✅ +0.09% coverage variation (-1.00%) Diff coverage ✅ 96.55% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (5d4c419) Report Missing Report Missing Report Missing Head commit (9de3b4e) 7824 (+51) 7411 (+55) 94.72% (+0.09%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#171) 58 56 96.55% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #171 +/- ##
==========================================
+ Coverage 93.42% 93.51% +0.08%
==========================================
Files 50 50
Lines 6222 6256 +34
==========================================
+ Hits 5813 5850 +37
+ Misses 261 257 -4
- Partials 148 149 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What
A module executed by
load()ran in a barestarlark.Threadthat inherited none of the Machine's execution context, so the step budget did not apply (a top-level loop in a loaded source module escaped the DoS guard — the budget only armed the main thread) andprint()in a loaded module went to stdout via a hardcodedfmt.Println, bypassing the Machine's print func.The load cache now builds its child thread from the Machine (
newLoadThread): the same print func, an independent copy of the step budget (with theMaxStepsExceededpanic), and the current run's context local. The budget is per-thread — enough to stop one runaway loaded module — not a shared aggregate counter.Because a loaded module can now panic (
OnMaxSteps), the cache'sget()recovers arounddoLoad: aloadedcompletion sentinel (notrecover() != nil, which is nil forpanic(nil)under go 1.19) decides whether it panicked; on the panic path it readies the entry with the error, drops it so a retry re-executes, and re-raises sorunInternalmaps it to a typed error. Without this a secondRunafter a budget hit blocked forever on the load cache's ready channel.Not covered here (documented follow-ups)
Lazy loaders built with
MakeModuleLoaderFromString/Reader/Filestill run on their own bare thread and inherit no budget/print (needs an API change); a context timeout cancels only the main thread, so a loaded module's pure-Starlark loop is bounded by the step budget, not the deadline.Tests
A loaded module hits the step budget; its
print()reaches the Machine print func; a second run after a budget panic (and after apanic(nil)loader) returns promptly instead of deadlocking. Full-race/vet/gofmt/Docker go1.19 clean.