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

Pickle Framing in C Pickle Accelerator #128853

Open
Legoclones opened this issue Jan 15, 2025 · 0 comments
Open

Pickle Framing in C Pickle Accelerator #128853

Legoclones opened this issue Jan 15, 2025 · 0 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@Legoclones
Copy link
Contributor

Legoclones commented Jan 15, 2025

Bug report

Bug description:

Is framing supposed to be implemented in the C Pickle Accelerator? PEP 3154 describes framing and some of the rules for it. It also says

A well-written C implementation doesn’t need additional memory copies for the framing layer, preserving general (un)pickling efficiency.

I'll be honest I don't super understand what this means, but it doesn't seem that it's saying it shouldn't be implemented in C. Framing seems to be implemented in the C accelerator when dumping/saving a pickle, but not loading. The load_frame function in fact only does one thing when it encounters the FRAME opcode - it only checks that frame_len bytes exist in the stream, then continues to do what it's doing. It never reads frame_len bytes into a buffer in memory to prevent lots of small reads by future opcodes (from what I can tell).

Because framing isn't really implemented in _pickle.c, there are a few pickles that fail to be loaded in pickle.py, but are just fine in _pickle.c. For example:

  • Python checks that up to frame_len bytes exist in the stream. If the argument for the FRAME opcode is larger than the number of bytes available, the Python implementation will simply read as many bytes as possible into the memory buffer and continue on, while the C implementation is looking for at a minimum frame_len bytes in the stream.
  • Breaking an opcode argument across frames (which is specifically prohibited by PEP 3154) is not checked in _pickle.c, and does not error out.
  • Overlapping frames are checked for in pickle.py but are not checked in _pickle.c and those pickles do not error out.

Is this intended behavior? I tried looking through old issues to see if there has been any discussion about this, but I haven't found any.

CPython versions tested on:

3.11

Operating systems tested on:

Linux

@Legoclones Legoclones added the type-bug An unexpected behavior, bug, or error label Jan 15, 2025
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: No status
Development

No branches or pull requests

1 participant