Python: Fix checkpoint state isolation - #7830
Conversation
Context on why this PR exists alongside #7697I opened issue #7683 and started working on a fix myself. While I was doing that, #7697 was opened independently and got ahead of my work. I read through that PR and its review discussion in detail, and the automated review surfaced a correctness concern with its I therefore finished my implementation using a pickle round-trip for isolation instead of I recognize that having two open PRs for the same issue isn't ideal, so I'm calling out the overlap explicitly rather than leaving it ambiguous. I'm happy to close or defer this PR if a maintainer prefers to continue with #7697; my goal is to make sure the checkpoint isolation fix lands without the |
Motivation & Context
Checkpoint state is currently not isolated at two boundaries:
State.export_state()/State.import_state()perform shallow copies, allowingnested mutable state to remain aliased between a checkpoint and live workflow
state. A resumed workflow can therefore mutate the checkpoint it was restored
from.
InMemoryCheckpointStoragedeep-copies onsave()but returns its internallystored checkpoint objects directly from
load(),list_checkpoints(), andget_latest(). Callers can therefore mutate the storage backend's internalstate through a returned checkpoint.
This can silently corrupt checkpoint snapshots without raising an exception.
This PR fixes both isolation gaps while preserving the existing checkpoint value
contract. Instead of
copy.deepcopy(), it uses a pickle round-trip for isolation,matching the serialization boundary already used by checkpoint persistence
backends. This is important because checkpoint values may be custom
pickle-serializable types that do not support
copy.deepcopy().Fixes #7683.
Description & Review Guide
What are the major changes?
isolate_checkpoint_value()helper using a pickle round-trip.State.export_state()to isolate nested mutable values when creatingcheckpoint state.
State.import_state()to isolate incoming checkpoint state beforemerging it into live state.
InMemoryCheckpointStorage.load(),list_checkpoints(), andget_latest()to return caller-owned checkpointobjects.
save()snapshot semantics by isolating the checkpoint at save time.CheckpointStorageprotocol.InMemoryCheckpointStorageandFileCheckpointStorage.intentionally not deepcopyable, ensuring the fix does not reproduce the
correctness regression identified during review of Python: Isolate checkpoint state from live workflow state across restoration and storage boundaries #7697.
What is the impact of these changes?
Checkpoint objects are now isolated from live workflow state and from storage
backend internals at the relevant ownership boundaries.
The change does not alter the checkpoint value contract to require
__deepcopy__support. Isolation uses the same pickle-serialization semanticsalready supported by checkpoint persistence.
The pickle round-trip is only performed at checkpoint export/import and storage
save/read boundaries; normal
State.get(),set(), andcommit()operationsare unchanged.
What do you want reviewers to focus on?
Please focus on:
State,WorkflowCheckpoint, andCheckpointStorage.copy.deepcopy(), particularly for custom pickle-serializable checkpointvalues.
repeated-read independence.
behavior for all checkpoint storage backends.
This PR incorporates the useful ownership-contract and conformance-test
direction discussed in #7697 and #7712, while avoiding the
copy.deepcopy()approach that was identified during review of #7697 as potentially breaking
previously valid pickle-serializable checkpoint values.
Related Issue
Fixes #7683
An existing PR, #7697, addresses the same issue but uses
copy.deepcopy()forisolation. Review identified that approach as a potential correctness regression
because pickle-serializable checkpoint values are not necessarily
deepcopyable.
This PR takes a different implementation approach using pickle round-trip
isolation and incorporates the relevant ownership-contract and conformance-test
feedback from #7697 and #7712.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.