fix[tool]: update VarAccess pickle implementation #4270
Conversation
|
can we add an explainer comment on why's this necessary? |
will get to it |
| # see https://github.com/python/cpython/issues/124937#issuecomment-2392227290 | ||
| def __reduce__(self): | ||
| dict_obj = {f.name: getattr(self, f.name) for f in fields(self)} | ||
| return self.__class__._produce, (dict_obj,) |
There was a problem hiding this comment.
You could also use the following trick if all fields can be passed as keyword arguments to constructor in subclasses:
| return self.__class__._produce, (dict_obj,) | |
| return partial(self.__class__, **dict_obj), () |
There was a problem hiding this comment.
interesting, is there a performance difference?
There was a problem hiding this comment.
It may be slower. The advantage is that you do not need to add private method _produce().
|
copying in some offline discussion with @cyberthirst about why the hash is safe even though the data structure is cyclic:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4270 +/- ##
==========================================
- Coverage 91.30% 90.07% -1.23%
==========================================
Files 110 110
Lines 15764 15770 +6
Branches 3461 3463 +2
==========================================
- Hits 14393 14205 -188
- Misses 935 1095 +160
- Partials 436 470 +34 ☔ View full report in Codecov by Sentry. |
What I did
fix a bug where unpickling
annotated_vyper_modulewould lead to a crash:a repro can be found at charles-cooper@2a5facf.
this is a blocker for tooling, for instance titanoboa relies on pickle/unpickle to cache CompilerData objects:
https://github.com/vyperlang/titanoboa/blob/86df8936654db20686410488738d7abaf165a4c9/boa/util/disk_cache.py#L65-L66
the underlying issue is that
pickle.loads()callsobj.__hash__()for objects that are keys in a hashed data structure - namely, dicts, sets and frozensets. this causes a crash when there is a cycle in the object graph, because the object is not fully instantiated at the time that__hash__()is called. this is a cpython bug, reported at python/cpython#124937.@serhiy-storchaka suggested the approach taken in this PR, which breaks the loop before pickling: python/cpython#124937 (comment).
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture