Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect pickling behavior in the c implemented classmethod descriptors #95196

Closed
UltimateLobster opened this issue Jul 24, 2022 · 2 comments
Closed
Labels
type-bug An unexpected behavior, bug, or error

Comments

@UltimateLobster
Copy link

UltimateLobster commented Jul 24, 2022

Bug report

Hello there, this is my first bug report so apologies if there's something I miss.

Consider the following code:

import pickle

classmethod_descriptor = dict.__dict__["fromkeys"]
unpickled_classmethod_descriptor = pickle.loads(pickle.dumps(classmethod_descriptor, protocol=5))

While it passes without raising any errors, classmethod_descriptor differs from unpickled_classmethod_descriptor. Instead of the original descriptor, the pickling process yields the function itself:

>>> classmethod_descriptor
<method 'fromkeys' of 'dict' objects>

>>> unpickled_classmethod_descriptor
<function dict.fromkeys(iterable, value=None, /)>

Manually calling the descriptor's __reduce_ex__ allows us to see the reason:

>>> classmethod_descriptor.__reduce_ex__(5)
(<function getattr>, (dict, 'fromkeys'))

This behavior differs from python implemented classmethods:

import pickle

class A:
    @classmethod
    def method(cls):
        pass

classmethod_descriptor = A.__dict__["method"]

# Any of these lines will raise "TypeError: cannot pickle 'classmethod' object"
pickle.dumps(classmethod_descriptor, protocol=5)
classmethod_descriptor.__reduce_ex__(5)

Since it fails with normal classmethods, I would expect it to fail with their c counterparts as well so their behavior would match.
Either that, or having both of them succeed during the pickling process, in which case, __reduce_ex__ should return a valid result in both cases.

The bug occurs with other c implemented class methods as well:

import pickle

from datetime import datetime

classmethod_descriptor = datetime.__dict__["now"]

# Returns False
pickle.loads(pickle.dumps(classmethod_descriptor, 5)) == classmethod_descriptor

# Returns True
pickle.loads(pickle.dumps(classmethod_descriptor, 5)) == datetime.now

Your environment

Python Versions:
I've tested it with both 3.10.5 and 3.11.04b
OS - Windows 10 Pro 64 bit

@UltimateLobster UltimateLobster added the type-bug An unexpected behavior, bug, or error label Jul 24, 2022
@UltimateLobster
Copy link
Author

#96162

Opened a PR to solve the issue

@serhiy-storchaka
Copy link
Member

Since this feature never worked correctly, and pickling of Python implemented classmethod descriptors and C and Python implemented staticmethod descriptors is not supported, and there are no common use cases for this (otherwise we would get such report years ago), and the correct implementation is not trivial if not use eval(), I thing that the best way to fix this is to disable it. There are no existing tests for this, and it looks that this code was added by mistake and was never tested.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Aug 29, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 5, 2022
…method descriptors (pythonGH-96383)

(cherry picked from commit 77f0249)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 5, 2022
…method descriptors (pythonGH-96383)

(cherry picked from commit 77f0249)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington added a commit that referenced this issue Oct 5, 2022
… descriptors (GH-96383)

(cherry picked from commit 77f0249)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington added a commit that referenced this issue Oct 5, 2022
… descriptors (GH-96383)

(cherry picked from commit 77f0249)

Co-authored-by: Serhiy Storchaka <[email protected]>
carljm added a commit to carljm/cpython that referenced this issue Oct 6, 2022
* main: (66 commits)
  pythongh-65961: Raise `DeprecationWarning` when `__package__` differs from `__spec__.parent` (python#97879)
  docs(typing): add "see PEP 675" to LiteralString (python#97926)
  pythongh-97850: Remove all known instances of module_repr() (python#97876)
  I changed my surname early this year (python#96671)
  pythongh-93738: Documentation C syntax (:c:type:<C type> -> :c:expr:<C type>) (python#97768)
  pythongh-91539: improve performance of get_proxies_environment  (python#91566)
  build(deps): bump actions/stale from 5 to 6 (python#97701)
  pythonGH-95172 Make the same version `versionadded` oneline (python#95172)
  pythongh-88050: Fix asyncio subprocess to kill process cleanly when process is blocked (python#32073)
  pythongh-93738: Documentation C syntax (Function glob patterns -> literal markup) (python#97774)
  pythongh-93357: Port test cases to IsolatedAsyncioTestCase, part 2 (python#97896)
  pythongh-95196: Disable incorrect pickling of the C implemented classmethod descriptors (pythonGH-96383)
  pythongh-97758: Fix a crash in getpath_joinpath() called without arguments (pythonGH-97759)
  pythongh-74696: Pass root_dir to custom archivers which support it (pythonGH-94251)
  pythongh-97661: Improve accuracy of sqlite3.Cursor.fetchone docs (python#97662)
  pythongh-87092: bring compiler code closer to a preprocessing-opt-assembler organisation (pythonGH-97644)
  pythonGH-96704: Add {Task,Handle}.get_context(), use it in call_exception_handler() (python#96756)
  pythongh-93738: Documentation C syntax (:c:type:`PyTypeObject*` -> :c:expr:`PyTypeObject*`) (python#97778)
  pythongh-97825: fix AttributeError when calling subprocess.check_output(input=None) with encoding or errors args (python#97826)
  Add re.VERBOSE flag documentation example (python#97678)
  ...
mpage pushed a commit to mpage/cpython that referenced this issue Oct 11, 2022
danigm added a commit to danigm/cloudpickle that referenced this issue Oct 28, 2022
This patch mvoes the builtin classmethod descriptor test to a different
one and mark it to skip for python >= 3.10.8. The classmethod descriptor
serializarion was removed from python in these releases:
https://docs.python.org/3.10/whatsnew/changelog.html#id3

More information in the github issue:
python/cpython#95196

Fix cloudpipe#485
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
Status: Done
2 participants