◐ Shell
reader mode source ↗
Skip to content

bpo-30570: Use Py_EnterRecursiveCall() in issubclass()#29048

Merged
gpshead merged 3 commits into
python:mainfrom
sweeneyde:issubclass_recursion
Oct 22, 2021
Merged

bpo-30570: Use Py_EnterRecursiveCall() in issubclass()#29048
gpshead merged 3 commits into
python:mainfrom
sweeneyde:issubclass_recursion

Conversation

@sweeneyde

@sweeneyde sweeneyde commented Oct 19, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

This is an alternative to GH-29017. This shouldn't affect performance too much because the recursion checks only occur once for each time a length>=2 __bases__ tuple is found (i.e. for multiple inheritance). Note that if we get stuck in the while (1) {...} loop, as long as the pathological __bases__ implementation is in Python, we can still interrupt with a KeyboardInterrupt.

https://bugs.python.org/issue30570

@gpshead gpshead self-requested a review October 19, 2021 05:26
@gpshead gpshead self-assigned this Oct 19, 2021
gpshead
gpshead approved these changes Oct 22, 2021
View reviewed changes

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

I added another test case (the specific example from the bug). Yours likely already covers it, just wanted it for pedantic reasons. I like this. Much nicer than mine. I was unaware of our new EnterRecursiveCall API pair. perfect! :)

@gpshead gpshead added the needs backport to 3.10 only security fixes label Oct 22, 2021
@gpshead gpshead mentioned this pull request Oct 22, 2021
Closed
@gpshead gpshead merged commit 423fa1c into python:main Oct 22, 2021
@miss-islington

miss-islington commented Oct 22, 2021

Copy link
Copy Markdown
Contributor

Thanks @sweeneyde for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 22, 2021
@sweeneyde @miss-islington
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (pythonGH-29048) …
8357734 
* Use Py_EnterRecursiveCall() in issubclass()

Reviewed-by: Gregory P. Smith <greg@krypto.org> [Google]
(cherry picked from commit 423fa1c)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
@bedevere-bot

bedevere-bot commented Oct 22, 2021

Copy link
Copy Markdown

GH-29175 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 22, 2021
miss-islington added a commit that referenced this pull request Oct 22, 2021
@miss-islington @sweeneyde
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048) …
f812fef 
* Use Py_EnterRecursiveCall() in issubclass()

Reviewed-by: Gregory P. Smith <greg@krypto.org> [Google]
(cherry picked from commit 423fa1c)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
@miss-islington

miss-islington commented Oct 22, 2021

Copy link
Copy Markdown
Contributor

Thanks @sweeneyde for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@bedevere-bot

bedevere-bot commented Oct 22, 2021

Copy link
Copy Markdown

GH-29178 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 22, 2021
@sweeneyde @miss-islington
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (pythonGH-29048) …
ec005f2 
* Use Py_EnterRecursiveCall() in issubclass()

Reviewed-by: Gregory P. Smith <greg@krypto.org> [Google]
(cherry picked from commit 423fa1c)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
@bedevere-bot

bedevere-bot commented Oct 23, 2021 •
edited
Loading

Copy link
Copy Markdown

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

Hi! The buildbot AMD64 Arch Linux Asan Debug 3.10 has failed when building commit f812fef.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/621/builds/332) and take a look at the build logs.
  4. Check if the failure is related to this commit (f812fef) or if it is a false positive.
  5. 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/621/builds/332

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

Click to see traceback logs 
Note: switching to 'f812fef2f8f12441ce559335645433c8124e7db5'.

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 f812fef2f8 [bpo-30570](https://bugs.python.org/issue30570): Use Py_EnterRecursiveCall() in issubclass() (GH-29048)
Switched to and reset branch '3.10'

renaming build/scripts-3.10/pydoc3 to build/scripts-3.10/pydoc3.10
renaming build/scripts-3.10/idle3 to build/scripts-3.10/idle3.10
renaming build/scripts-3.10/2to3 to build/scripts-3.10/2to3-3.10

renaming build/scripts-3.10/pydoc3 to build/scripts-3.10/pydoc3.10
renaming build/scripts-3.10/idle3 to build/scripts-3.10/idle3.10
renaming build/scripts-3.10/2to3 to build/scripts-3.10/2to3-3.10

renaming build/scripts-3.10/pydoc3 to build/scripts-3.10/pydoc3.10
renaming build/scripts-3.10/idle3 to build/scripts-3.10/idle3.10
renaming build/scripts-3.10/2to3 to build/scripts-3.10/2to3-3.10
test_flock (__main__.FNTLEINTRTest) ... ok
test_lockf (__main__.FNTLEINTRTest) ... ok
test_read (__main__.OSEINTRTest) ... ok
test_wait (__main__.OSEINTRTest) ... ok
test_wait3 (__main__.OSEINTRTest) ... ok
test_wait4 (__main__.OSEINTRTest) ... ok
test_waitpid (__main__.OSEINTRTest) ... ok
test_write (__main__.OSEINTRTest) ... ok
test_devpoll (__main__.SelectEINTRTest) ... skipped 'need select.devpoll'
test_epoll (__main__.SelectEINTRTest) ... ok
test_kqueue (__main__.SelectEINTRTest) ... skipped 'need select.kqueue'
test_poll (__main__.SelectEINTRTest) ... ok
test_select (__main__.SelectEINTRTest) ... ok
test_sigtimedwait (__main__.SignalEINTRTest) ... ok
test_sigwaitinfo (__main__.SignalEINTRTest) ... ok
test_accept (__main__.SocketEINTRTest) ... ok
test_open (__main__.SocketEINTRTest) ... ok
test_os_open (__main__.SocketEINTRTest) ... ok
test_recv (__main__.SocketEINTRTest) ... ok
test_recvmsg (__main__.SocketEINTRTest) ... ok
test_send (__main__.SocketEINTRTest) ... ok
test_sendall (__main__.SocketEINTRTest) ... ok
test_sendmsg (__main__.SocketEINTRTest) ... ok
test_sleep (__main__.TimeEINTRTest) ... ok

----------------------------------------------------------------------
Ran 24 tests in 7.352s

OK (skipped=2)
test_winsound skipped -- No module named 'winsound'
test_zipfile64 skipped -- test requires loads of disk-space bytes and a long time to run
test_msilib skipped -- No module named '_msi'
test_kqueue skipped -- test works only on BSD
test_winreg skipped -- No module named 'winreg'
test_winconsoleio skipped -- test only relevant on win32
test_startfile skipped -- object <module 'os' from '/buildbot/buildarea/3.10.pablogsal-arch-x86_64.asan_debug/build/Lib/os.py'> has no attribute 'startfile'
test_badargs (__main__.GeneralTest) ... ok
test_bound_methods (__main__.GeneralTest) ... ok
test_clear (__main__.GeneralTest) ... ok
test_exit (__main__.GeneralTest) ... ok
test_order (__main__.GeneralTest) ... ok
test_raise (__main__.GeneralTest) ... ok
test_raise_unnormalized (__main__.GeneralTest) ... ok
test_stress (__main__.GeneralTest) ... ok
test_unregister (__main__.GeneralTest) ... ok

----------------------------------------------------------------------
Ran 9 tests in 0.003s

OK
test_tk skipped -- Tk unavailable due to TclError: couldn't connect to display ":99"
test_ioctl skipped -- Unable to open /dev/tty
test_devpoll skipped -- test works only on Solaris OS family
test_ttk_guionly skipped -- Tk unavailable due to TclError: couldn't connect to display ":99"
test_tix skipped -- Tk unavailable due to TclError: couldn't connect to display ":99"
make: *** [Makefile:1256: buildbottest] Terminated

Cannot open file '/buildbot/buildarea/3.10.pablogsal-arch-x86_64.asan_debug/build/test-results.xml' for upload

@Fidget-Spinner Fidget-Spinner mentioned this pull request Oct 28, 2021
Merged
@sweeneyde sweeneyde mentioned this pull request Oct 28, 2021
Merged
ambv pushed a commit that referenced this pull request Nov 4, 2021
@miss-islington @sweeneyde
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048) (GH… …
1e29dce 
…-29178)

* Use Py_EnterRecursiveCall() in issubclass()

Reviewed-by: Gregory P. Smith <greg@krypto.org> [Google]
(cherry picked from commit 423fa1c)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
@sweeneyde sweeneyde deleted the issubclass_recursion branch December 19, 2021 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes

Assignees

@gpshead gpshead

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sweeneyde @miss-islington @bedevere-bot @gpshead @the-knights-who-say-ni