classification
Title: Opt out serialization/deserialization for heap type
Type: behavior Stage: resolved
Components: C API Versions: Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: corona10, pitrou, serhiy.storchaka, shihai1991, vstinner
Priority: normal Keywords: patch

Created on 2020-06-20 13:17 by corona10, last changed 2020-10-27 04:22 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 21002 merged corona10, 2020-06-20 13:47
PR 21030 merged corona10, 2020-06-21 09:58
PR 22189 closed corona10, 2020-09-10 14:48
PR 22870 merged serhiy.storchaka, 2020-10-21 20:47
PR 22963 merged serhiy.storchaka, 2020-10-25 08:10
Messages (10)
msg371937 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-06-20 13:17
See https://bugs.python.org/issue40077#msg371813

We noticed that heap type has different behavior about serialization/deserialization.

Basically it can occur the regression issues.

Two things needed.
1. opt out serialization/deserialization for converted modules.
2. Add unit tests to check whether their serialization is blocked.
3. If the module is already ported to 3.9 backport patch is needed.
Long term
- Add the object.reduce() and/or update pickle can be smarter
msg371992 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-06-21 09:45
New changeset 6989af0bc7ea1e9a1acea16794e6f723d7b44110 by Dong-hee Na in branch 'master':
bpo-41052: Opt out serialization/deserialization for _random.Random (GH-21002)
https://github.com/python/cpython/commit/6989af0bc7ea1e9a1acea16794e6f723d7b44110
msg371993 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-06-21 10:33
New changeset 814b07bf814a804f60b897d18f1dbb5578b2c7fd by Dong-hee Na in branch '3.9':
[3.9] bpo-41052: Opt out serialization/deserialization for _random.Random (GH-21002). (GH-21030)
https://github.com/python/cpython/commit/814b07bf814a804f60b897d18f1dbb5578b2c7fd
msg376951 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-09-15 20:10
There are other heap types implemented in C in the stdlib and third-party libraries for which pickling with protocols 0 and 1 works incorrectly. It would be better to fix copyreg._reduce_ex() instead of disabling pickling for these types one by one.

From PEP 307:


> Let D be the class on the object to be pickled. First, find the nearest base class that is implemented in C (either as a built-in type or as a type defined by an extension class). Call this base class B, and the class of the object to be pickled D. Unless B is the class 'object', instances of class B must be picklable, either by having built-in support (as defined in the above three bullet points), or by having a non-default __reduce__ implementation. B must not be the same class as D (if it were, it would mean that D is not implemented in Python).

The problem is with determining which class is implemented in C. The current code implies that heap types are implemented in Python, and static types are implemented in C. It is not always true, because some heap types can be implemented in C.
msg379214 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-10-21 15:42
> It would be better to fix copyreg._reduce_ex() instead of disabling pickling for these types one by one

I agree :)
msg379223 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2020-10-21 17:11
At some point, pickle protocols 0 and 1 should be deprecated and slated for removal.  Protocol 2 is 17 years ago already, and protocol 3 has become the default in Python 3.

That would probably be a better use of contributor time than trying to make heap types compatible with those obsolete protocols.
msg379251 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-10-21 20:52
I came to the same conclusion after trying to fix it. But we cannot just do it now. We have to fix bugs in Python 3.9 and support protocols 0 and 1 for some deprecation period.

Also protocols 0 and 1 are used in some third-party implementations for other programming languages, so they can be used for interoperability with other languages.

The proposed patch adds additional check. It uses the fact that the __new__ attribute for classes implemented in C is a bultin function with __self__ pointing to this class (it is created in tp_new_wrapper()).
msg379532 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-10-24 18:14
New changeset 8cd1dbae32d9303caac3a473d3332f17bc98c921 by Serhiy Storchaka in branch 'master':
bpo-41052: Fix pickling heap types implemented in C with protocols 0 and 1 (GH-22870)
https://github.com/python/cpython/commit/8cd1dbae32d9303caac3a473d3332f17bc98c921
msg379585 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-10-25 15:56
New changeset 0aaecb30483e98fc29c1f4253967d05da092f0c5 by Serhiy Storchaka in branch '3.9':
[3.9] bpo-41052: Fix pickling heap types implemented in C with protocols 0 and 1 (GH-22870). (GH-22963)
https://github.com/python/cpython/commit/0aaecb30483e98fc29c1f4253967d05da092f0c5
msg379735 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-10-27 04:22
Thanks for the fix Serhiy!
History
Date User Action Args
2020-10-27 04:22:53vstinnersetmessages: + msg379735
2020-10-25 16:09:30serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-10-25 15:56:27serhiy.storchakasetmessages: + msg379585
2020-10-25 08:10:59serhiy.storchakasetpull_requests: + pull_request21880
2020-10-24 18:14:26serhiy.storchakasetmessages: + msg379532
2020-10-21 20:52:33serhiy.storchakasetmessages: + msg379251
2020-10-21 20:47:44serhiy.storchakasetpull_requests: + pull_request21812
2020-10-21 17:11:37pitrousetnosy: + pitrou
messages: + msg379223
2020-10-21 15:42:18corona10setmessages: + msg379214
2020-09-15 20:10:45serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg376951
2020-09-10 14:48:38corona10setpull_requests: + pull_request21250
2020-06-21 10:33:14corona10setmessages: + msg371993
2020-06-21 09:58:01corona10setpull_requests: + pull_request20201
2020-06-21 09:45:05corona10setmessages: + msg371992
2020-06-20 13:47:45corona10setkeywords: + patch
stage: patch review
pull_requests: + pull_request20177
2020-06-20 13:45:04corona10setnosy: + shihai1991
2020-06-20 13:17:29corona10create