gh-103092: Isolate `_pickle` #102982

erlend-aasland · 2023-03-23T23:30:29Z

Issue: Isolate Stdlib Extension Modules #103092

erlend-aasland · 2023-03-23T23:34:11Z

Based on previous work by Mohamed Koubaa:

bpo-1635741: port pickle to multi-phase init (PEP 489) #23188

bedevere-bot · 2023-03-24T08:40:10Z

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 0551d52 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

Misc/NEWS.d/next/Core and Builtins/2020-07-04-09-04-41.bpo-1635741.ZsP31Y.rst

…5741.ZsP31Y.rst

Modules/_pickle.c

erlend-aasland · 2023-04-02T23:17:09Z

FYI, I made a PoC alternative variant where I store the state in the pickler and unpickler objects. It makes for a much nicer diff, but I had to do too much fishy stuff to make it work. You can take a look here: https://github.com/erlend-aasland/cpython/pull/new/isolate-pickle-alt

Modules/_pickle.c

…plemented

Modules/_pickle.c

bedevere-bot · 2023-04-03T14:51:12Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Modules/_pickle.c

kumaraditya303 · 2023-04-04T10:10:06Z

Merged, will start reviewing others in queue.

erlend-aasland · 2023-04-04T10:43:56Z

Thanks for the review, Kumar, and thanks for the initial work, Mohamed!

bedevere-bot · 2023-04-04T10:55:44Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 Fedora Stable 3.x has failed when building commit c00dcf0.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/125/builds/3374) and take a look at the build logs.
Check if the failure is related to this commit (c00dcf0) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/125/builds/3374

Failed tests:

test_asyncio

Summary of the results of the build (if available):

==

Click to see traceback logs

remote: Enumerating objects: 18, done.        
remote: Counting objects:   5% (1/17)        
remote: Counting objects:  11% (2/17)        
remote: Counting objects:  17% (3/17)        
remote: Counting objects:  23% (4/17)        
remote: Counting objects:  29% (5/17)        
remote: Counting objects:  35% (6/17)        
remote: Counting objects:  41% (7/17)        
remote: Counting objects:  47% (8/17)        
remote: Counting objects:  52% (9/17)        
remote: Counting objects:  58% (10/17)        
remote: Counting objects:  64% (11/17)        
remote: Counting objects:  70% (12/17)        
remote: Counting objects:  76% (13/17)        
remote: Counting objects:  82% (14/17)        
remote: Counting objects:  88% (15/17)        
remote: Counting objects:  94% (16/17)        
remote: Counting objects: 100% (17/17)        
remote: Counting objects: 100% (17/17), done.        
remote: Compressing objects:  12% (1/8)        
remote: Compressing objects:  25% (2/8)        
remote: Compressing objects:  37% (3/8)        
remote: Compressing objects:  50% (4/8)        
remote: Compressing objects:  62% (5/8)        
remote: Compressing objects:  75% (6/8)        
remote: Compressing objects:  87% (7/8)        
remote: Compressing objects: 100% (8/8)        
remote: Compressing objects: 100% (8/8), done.        
remote: Total 18 (delta 9), reused 11 (delta 9), pack-reused 1        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to 'c00dcf0e381a090f7e905f887a98bbf63c88af5a'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at c00dcf0e38 gh-103092: Isolate `_pickle` module (#102982)
Switched to and reset branch 'main'

Timeout (0:15:00)!
Thread 0x0000ffffbbdd4020 (most recent call first):
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/test_asyncio/test_tasks.py", line 137 in __del__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/test_asyncio/test_tasks.py", line 142 in test_task_del_collect
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/case.py", line 589 in _callTestMethod
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/case.py", line 634 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/case.py", line 690 in __call__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 122 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 122 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 122 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 122 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 122 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/unittest/runner.py", line 240 in run
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/support/__init__.py", line 1106 in _run_suite
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/support/__init__.py", line 1232 in run_unittest
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/runtest.py", line 281 in _test_module
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/runtest.py", line 317 in _runtest_inner2
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/runtest.py", line 360 in _runtest_inner
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/runtest.py", line 235 in _runtest
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/runtest.py", line 265 in runtest
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/main.py", line 353 in rerun_failed_tests
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/main.py", line 756 in _main
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/main.py", line 711 in main
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/libregrtest/main.py", line 775 in main
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/test/__main__.py", line 2 in <module>
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/runpy.py", line 88 in _run_code
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/Lib/runpy.py", line 198 in _run_module_as_main
make: *** [Makefile:1950: buildbottest] Error 1

Cannot open file '/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64/build/build_oot/test-results.xml' for upload

Co-authored-by: Mohamed Koubaa <koubaa.m@gmail.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

koubaa and others added 12 commits March 23, 2023 22:27

try to port pickle to multi-stage init

88455f2

remove global state access

7a6189a

finish port

ce32a69

Convert PData type to heap type and add it to state

bffb49b

Convert PicklerMemoProxyType to heap type and add it to state

5fcce82

WIP

3203188

Fixup dealloc and traverse functions

f1ce8fb

State is unused in AC

a6679ff

Adjust NEWS

35025d2

Update globals-to-fix

d8efd22

Adjust get-state helpers

9fcc0d9

Reduce diff size; remove stylic changes

add3ba5

erlend-aasland requested a review from kumaraditya303 March 23, 2023 23:30

bedevere-bot added the awaiting core review label Mar 23, 2023

erlend-aasland changed the title ~~isolate pickle~~ WIP: Isolate _pickle Mar 23, 2023

erlend-aasland added 4 commits March 24, 2023 00:35

Nit

b915f06

Revert type name changes

223e143

fixup! Revert type name changes

288a764

Add CREATE_TYPE helper

0551d52

erlend-aasland added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 24, 2023

bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 24, 2023

erlend-aasland changed the title ~~WIP: Isolate _pickle~~ [PoC] Isolate _pickle Mar 24, 2023

erlend-aasland changed the title ~~[PoC] Isolate _pickle~~ gh-103092: Isolate _pickle Mar 28, 2023

bedevere-bot mentioned this pull request Mar 28, 2023

Isolate Stdlib Extension Modules #103092

Open

25 tasks

kumaraditya303 reviewed Apr 1, 2023

View reviewed changes

Misc/NEWS.d/next/Core and Builtins/2020-07-04-09-04-41.bpo-1635741.ZsP31Y.rst Outdated Show resolved Hide resolved

Update Misc/NEWS.d/next/Core and Builtins/2020-07-04-09-04-41.bpo-163…

ae0f6af

…5741.ZsP31Y.rst

kumaraditya303 reviewed Apr 1, 2023

View reviewed changes

Modules/_pickle.c Show resolved Hide resolved

kumaraditya303 reviewed Apr 3, 2023

View reviewed changes

Modules/_pickle.c Show resolved Hide resolved

kumaraditya303 reviewed Apr 3, 2023

View reviewed changes

Modules/_pickle.c Outdated Show resolved Hide resolved

erlend-aasland marked this pull request as ready for review April 3, 2023 13:24

erlend-aasland added 3 commits April 3, 2023 15:28

Pull in main

82fe129

Address review: in memo proxy types set tp_hash to PyObject_HashNotIm…

6c1621e

…plemented

Address review: find state by module def in get/set methods

ffc25ad

kumaraditya303 requested changes Apr 3, 2023

View reviewed changes

Modules/_pickle.c Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting core review labels Apr 3, 2023

Address review

e363fed

erlend-aasland requested a review from kumaraditya303 April 3, 2023 15:33

kumaraditya303 reviewed Apr 3, 2023

View reviewed changes

Modules/_pickle.c Outdated Show resolved Hide resolved

kumaraditya303 reviewed Apr 3, 2023

View reviewed changes

Modules/_pickle.c Outdated Show resolved Hide resolved

erlend-aasland added 2 commits April 3, 2023 22:25

Pull in main

233e102

Address review: make memo types immutable and add tests

bb397fe

erlend-aasland requested a review from kumaraditya303 April 3, 2023 20:46

kumaraditya303 approved these changes Apr 4, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Apr 4, 2023

kumaraditya303 merged commit c00dcf0 into python:main Apr 4, 2023

bedevere-bot removed the awaiting merge label Apr 4, 2023

kumaraditya303 deleted the isolate-pickle branch April 4, 2023 10:09

Yhg1s mentioned this pull request Apr 4, 2023

test_pickle/test_pickletools fail when running tests sequentially. #103247

Closed

warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023

pythongh-103092: Isolate _pickle module (python#102982)

dbbf2b0

Co-authored-by: Mohamed Koubaa <koubaa.m@gmail.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

encukou mentioned this pull request Mar 28, 2024

bpo-1635741: In pickle module, inject module state from class methods #23304

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-103092: Isolate `_pickle` #102982

gh-103092: Isolate `_pickle` #102982

erlend-aasland commented Mar 23, 2023 •

edited by bedevere-bot

Loading

erlend-aasland commented Mar 23, 2023

bedevere-bot commented Mar 24, 2023

erlend-aasland commented Apr 2, 2023

bedevere-bot commented Apr 3, 2023

kumaraditya303 commented Apr 4, 2023

erlend-aasland commented Apr 4, 2023

bedevere-bot commented Apr 4, 2023

gh-103092: Isolate _pickle #102982

gh-103092: Isolate _pickle #102982

Conversation

erlend-aasland commented Mar 23, 2023 • edited by bedevere-bot Loading

erlend-aasland commented Mar 23, 2023

bedevere-bot commented Mar 24, 2023

erlend-aasland commented Apr 2, 2023

bedevere-bot commented Apr 3, 2023

kumaraditya303 commented Apr 4, 2023

erlend-aasland commented Apr 4, 2023

bedevere-bot commented Apr 4, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

gh-103092: Isolate `_pickle` #102982

gh-103092: Isolate `_pickle` #102982

erlend-aasland commented Mar 23, 2023 •

edited by bedevere-bot

Loading