Skip to content

fix(sdk-python): detach Run finalizer on finish/exit (fixes #97) - #102

Open
Srinivasan8888 wants to merge 1 commit into
evo-hq:mainfrom
Srinivasan8888:fix/issue-97-sdk-finalizer
Open

Srinivasan8888 wants to merge 1 commit into
evo-hq:mainfrom
Srinivasan8888:fix/issue-97-sdk-finalizer

Conversation

@Srinivasan8888

Copy link
Copy Markdown

Problem

The Python SDK keeps a process-wide registry (_ACTIVE_RUNS) so two Runs with the same experiment_id can't be live at once. Each Run also arms weakref.finalize(self, _release_active_run, self._experiment_id) as a safety net for a leaked run.

finish() / __exit__ release the slot but never detach() the finalizer. Since the registry is keyed only by experiment_id, GC of an already-finished Run fires its stale finalizer and frees the slot of a different, still-active Run that reused the id:

r1 = Run(experiment_id="x"); r1.finish()   # slot freed
r2 = Run(experiment_id="x")                 # allowed; r2 live, never finished
del r1; gc.collect()                        # r1's stale finalizer clears "x"
r3 = Run(experiment_id="x")                 # SUCCEEDS — guard defeated

r2 and r3 then both run and collide writing EVO_RESULT_PATH — the exact late-failure the registry was added to prevent.

Fix

detach() the finalizer immediately after the slot is released, in both finish()'s finally and __exit__'s exception path. The leaked-run safety net (finalizer fires for a never-finished Run) is unchanged.

Tests

tests/unit/test_sdk_run_finalizer.py (TDD, failing first):

  • GC of a finished Run does NOT free a live Run's slot (via finish() and via with)
  • a leaked (never-finished) Run still releases its slot on GC

Existing sdk/python/test passes.

Fixes #97.

…ouble-Run guard

Each Run arms weakref.finalize(self, _release_active_run, experiment_id) as
a safety net for leaked (never-finished) runs. finish()/__exit__ released
the registry slot but never detached that finalizer, so when a *finished*
Run was later garbage-collected its finalizer fired and discarded the
experiment_id from _ACTIVE_RUNS -- even when that id now belonged to a
different, still-active Run. That silently freed the live Run's slot and
let a third Run for the same id start, defeating the guard the registry
exists to enforce.

Detach the finalizer right after releasing the slot in both finish() and
the __exit__ exception path. The leaked-run safety net is unchanged.

Fixes evo-hq#97.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

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.

Python SDK: stale weakref.finalize frees a live Run's registry slot, defeating the double-Run guard

1 participant