Skip to content

fix: persist the sandbox namespace with a single dill dump - #101

Open
YoanSallami wants to merge 1 commit into
mainfrom
fix/sandbox-persist-single-dump
Open

fix: persist the sandbox namespace with a single dill dump#101
YoanSallami wants to merge 1 commit into
mainfrom
fix/sandbox-persist-single-dump

Conversation

@YoanSallami

Copy link
Copy Markdown
Contributor

Problem

MirageSandbox persists the REPL namespace between run calls by pickling it with dill. It did so item by item, which is quadratic: dill pickles a function together with a copy of its globals, so N sandbox-defined functions cost N full-namespace pickles.

Measured on this branch's test snippet (120 defs in one run):

old new
persist 120 functions ~3.8 s ~0.4 s
a few hundred functions RecursionError, namespace lost fine

A second, sharper bug hid in the same loop: one unpicklable value dropped every sandbox-defined function. The offending object was reachable through the shared globals each function copied, so dill.dumps(item) failed for all of them:

sandbox.run("gen = (i for i in range(3))\ndef survivor(x):\n    return x * 3")
sandbox.run("print(survivor(5))")  # NameError: name 'survivor' is not defined

Fix

  • One dill.dumps of the whole namespace, which memoizes the shared globals once.
  • The per-item filter stays as a fallback for a namespace holding something unpicklable (a generator, an open socket), now pickling with recurse=True so a function carries only the globals it references rather than the whole namespace. Restored functions are re-homed onto the live namespace on the next run, so the reduced globals are never observable.
  • The state file is written to a sibling temp file and os.replaced, so a run killed mid-write (host timeout) cannot leave a truncated state file.

Tests

Two regression tests in mirage_sandbox_test.py: test_many_functions_persist and test_unpicklable_value_does_not_drop_the_namespace (the latter fails on the old code with the NameError above).

uv run pytest synalinks/src/sandboxes/mirage_sandbox_test.py → 124 passed, 9 skipped. uvx ruff check clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GfeWSiwmNYwhbPNLR75mer

Pickling the namespace item by item was quadratic: dill pickles a
function together with a copy of its globals, so N sandbox-defined
functions cost N full-namespace pickles (120 definitions ~3.8 s, a few
hundred hit RecursionError and lost the namespace outright). One
dill.dumps of the whole namespace memoizes the shared globals: the same
120 definitions now persist in ~0.4 s.

The per-item filter stays as a fallback for a namespace holding
something unpicklable (a generator, an open socket), and now pickles
with recurse=True so a function carries only the globals it references.
Without that, one unpicklable value dragged every sandbox-defined
function down with it: the offending object was reachable through the
shared globals every function copied. Restored functions are re-homed
onto the live namespace on the next run, so the reduced globals are
never observable.

Also write the state through a sibling temp file and os.replace, so a
run killed mid-write (host timeout) cannot leave a truncated state file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GfeWSiwmNYwhbPNLR75mer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant