Save object proxies as placeholders without touching them - #12
Merged
Conversation
An object proxy such as an rpyc netref forwards every instance operation to a remote peer: attribute reads, repr, str, isinstance through __class__, and pickling through __reduce_ex__ (which is how rpyc.classic.obtain fetches a value). Each is a synchronous request that blocks for the peer's full timeout when the connection is broken, and a proxy in a crashing frame's locals is common because a dead peer is often why the test failed. The save pickled every frame value to check it round-trips (one request), then built the placeholder with repr (a second request, made inside the except handler, so its failure escaped and lost the whole dump). Measured against a peer that never answers, that was two timeouts per proxy and then no dump at all. save_traceback now takes ``proxy_types``: classes, or fully-qualified class names such as "rpyc.core.netref.BaseNetref" so a package need not be importable, whose instances are recognised from ``type(value).__mro__`` alone - the one thing guaranteed local for any object - and written as a placeholder naming the class and identity. The check runs in ``_filter_dict`` before the round-trip attempt and in the pickler's ``reducer_override`` ahead of its isinstance test, so proxies nested in containers and in an exception's args or attributes are covered too. ``DEFAULT_PROXY_TYPES`` names rpyc's base class out of the box. Against a real rpyc server, live or wedged, the save now makes no remote calls. Independently of registration, a placeholder can no longer lose the dump: the unpicklable-value placeholder falls back to the bare object repr when repr raises, and the unpicklable-exception fallback does the same for str. The exception fallback also catches BaseException, as the value check already did. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHo9JUkNpsczwn1iKtduXL
ProxyTypes held two frozensets and one method: a closure written as a class. proxy_matcher() now normalises and validates the entries once and returns a plain ``ProxyMatcher`` predicate, which the save threads through the frame filter, the pickler and the dump helpers as ``is_proxy``. An empty registry returns a shared always-false function, so the pickler's default costs nothing. A predicate is also the more general contract: a proxy family that cannot be recognised by class could be handed to the same parameter without the API changing shape. The one property that must survive any replacement is the same as before - the predicate consults only the value's type, since it runs ahead of the pickler's own isinstance(). No behaviour change; the tests are untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHo9JUkNpsczwn1iKtduXL
The proxy support landed with prose that restated the same mechanism in the README, the docstrings and the test module. Each now says it once: why a proxy must be recognised by type alone, what an entry may be, and where the placeholder is written. No behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVK1CMMzvu4taVdBpbbPbH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An object proxy such as an rpyc netref forwards every instance operation to a remote peer: attribute reads, repr, str, isinstance through class, and pickling through reduce_ex (which is how rpyc.classic.obtain fetches a value). Each is a synchronous request that blocks for the peer's full timeout when the connection is broken, and a proxy in a crashing frame's locals is common because a dead peer is often why the test failed. The save pickled every frame value to check it round-trips (one request), then built the placeholder with repr (a second request, made inside the except handler, so its failure escaped and lost the whole dump). Measured against a peer that never answers, that was two timeouts per proxy and then no dump at all.
save_traceback now takes
proxy_types: classes, or fully-qualified class names such as "rpyc.core.netref.BaseNetref" so a package need not be importable, whose instances are recognised fromtype(value).__mro__alone - the one thing guaranteed local for any object - and written as a placeholder naming the class and identity. The check runs in_filter_dictbefore the round-trip attempt and in the pickler'sreducer_overrideahead of its isinstance test, so proxies nested in containers and in an exception's args or attributes are covered too.DEFAULT_PROXY_TYPESnames rpyc's base class out of the box. Against a real rpyc server, live or wedged, the save now makes no remote calls.Independently of registration, a placeholder can no longer lose the dump: the unpicklable-value placeholder falls back to the bare object repr when repr raises, and the unpicklable-exception fallback does the same for str. The exception fallback also catches BaseException, as the value check already did.
Claude-Session: https://claude.ai/code/session_01HHo9JUkNpsczwn1iKtduXL