GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-97591: In `Exception.__setstate__()`, acquire strong reference before calling `tp_hash` slot. by ofey404 · Pull Request #97700 · python/cpython

Conversation

ofey404 commented Oct 1, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Added an Py_INCREF / Py_DECREF pair around the call of tp_hash slot in Exception.__setstate__().

If the last reference of dict value is cleared (by dict.clear()), during the __setstate__ method call, it would cause gc crash.

A script to reproduce this is in #97591 :

import gc

class Key(str):
    def __hash__(self):
        d.clear()
        return 0

class Value(str):
    pass

d = {}
d[Key()] = Value()

e = Exception()
e.__setstate__(d)

gc.collect() # Segfault happens here

Similiar issue: #92930

Issue: Setting state of an exception object with a dic crashes Python 3.8.14 #97591

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          patch BaseException_setstate and its test.

859e993

ofey404 requested a review from iritkatriel as a code owner

October 1, 2022 08:53

bedevere-bot added the awaiting review label

Oct 1, 2022

blurb-it Bot and others added 2 commits

October 1, 2022 08:55


          📜🤖 Added by blurb_it.

d86ff33


          Patchcheck.

kumaraditya303 added type-bug

An unexpected behavior, bug, or error

needs backport to 3.10 needs backport to 3.11

only security fixes

type-crash A hard crash of the interpreter, possibly with a core dump labels

Oct 1, 2022

gvanrossum approved these changes

Oct 1, 2022

View reviewed changes

gvanrossum left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LG, I just think you could add your name to the NEWS item.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot added awaiting merge and removed awaiting review labels

Oct 1, 2022


          Update Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issu…

b5dfd41

…e-97591.pw6kkH.rst

Co-authored-by: Guido van Rossum <[email protected]>

ofey404 commented Oct 2, 2022

Copy link

Copy Markdown

Contributor Author

@kumaraditya303 I notice the “back port to 3.10/11” tag, and I wonder how to do the back port? If it’s proper I’d like to do it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum approved these changes

Oct 2, 2022

View reviewed changes

gvanrossum left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM.

Regarding the backport, this is usually taken care of automatically by the Miss-Islington bot. Only if there are git merge problems the bot can't do it, a manual backport is required. In that case you're welcome to try it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gvanrossum merged commit d639438 into python:main

Oct 2, 2022

miss-islington commented Oct 2, 2022

Copy link

Copy Markdown

Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed awaiting merge needs backport to 3.11 only security fixes labels

Oct 2, 2022

bedevere-bot commented Oct 2, 2022

Copy link

Copy Markdown

GH-97716 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the needs backport to 3.10 label

Oct 2, 2022

bedevere-bot commented Oct 2, 2022

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 2, 2022


          pythongh-97591: In Exception.__setstate__() acquire strong referenc…

06b67fb

…es before calling `tp_hash` slot (pythonGH-97700)

(cherry picked from commit d639438)

Co-authored-by: Ofey Chan <[email protected]>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 2, 2022


          pythongh-97591: In Exception.__setstate__() acquire strong referenc…

c35e9d7

…es before calling `tp_hash` slot (pythonGH-97700)

(cherry picked from commit d639438)

Co-authored-by: Ofey Chan <[email protected]>

gvanrossum commented Oct 2, 2022

Copy link

Copy Markdown

Member

You're lucky, the backports happened automatically. They'll be merged as soon as the tests pass (since I pre-approved them).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington added a commit that referenced this pull request

Oct 2, 2022


          gh-97591: In Exception.__setstate__() acquire strong references bef…

c6fcbb4

…ore calling `tp_hash` slot (GH-97700)

(cherry picked from commit d639438)

Co-authored-by: Ofey Chan <[email protected]>

miss-islington added a commit that referenced this pull request

Oct 2, 2022


          gh-97591: In Exception.__setstate__() acquire strong references bef…

dbde686

…ore calling `tp_hash` slot (GH-97700)

(cherry picked from commit d639438)

Co-authored-by: Ofey Chan <[email protected]>

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request

Oct 2, 2022


          pythongh-97591: In Exception.__setstate__() acquire strong referenc…

e401b65

…es before calling `tp_hash` slot (python#97700)

carljm added a commit to carljm/cpython that referenced this pull request

Oct 3, 2022


          Merge branch 'main' into dictwatch

e83abac

* main: (2069 commits)
  pythongh-96512: Move int_max_str_digits setting to PyConfig (python#96944)
  pythongh-94808: Coverage: Check picklablability of calliter (python#95923)
  pythongh-94808: Add test coverage for PyObject_HasAttrString (python#96627)
  pythongh-94732: Fix KeyboardInterrupt race in asyncio run_forever() (python#97765)
  Fix typos in `bltinmodule.c`. (pythonGH-97766)
  pythongh-94808: `_PyLineTable_StartsLine` was not used (pythonGH-96609)
  pythongh-97681: Remove Tools/demo/ directory (python#97682)
  Fix typo in unittest docs (python#97742)
  pythongh-97728: Argument Clinic: Fix uninitialized variable in the Py_UNICODE converter (pythonGH-97729)
  pythongh-95913: Fix PEP number in PEP 678 What's New ref label (python#97739)
  pythongh-95913: Copyedit/improve New Modules What's New section (python#97721)
  pythongh-97740: Fix bang in Sphinx C domain ref target syntax (python#97741)
  pythongh-96819: multiprocessing.resource_tracker: check if length of pipe write <= 512 (python#96890)
  pythongh-97706: multiprocessing tests: Delete unused variable `rand` (python#97707)
  pythonGH-85447: Clarify docs about awaiting future multiple times (python#97738)
  [docs] Update logging cookbook with recipe for using a logger like an output… (pythonGH-97730)
  pythongh-97607: Fix content parsing in the impl-detail reST directive (python#97652)
  pythongh-95975: Move except/*/finally ref labels to more precise locations (python#95976)
  pythongh-97591: In `Exception.__setstate__()` acquire strong references before calling `tp_hash` slot (python#97700)
  pythongh-95588: Drop the safety claim from `ast.literal_eval` docs. (python#95919)
  ...

Reviewers

gvanrossum

gvanrossum approved these changes

iritkatriel

Awaiting requested review from iritkatriel

iritkatriel is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

type-bug

An unexpected behavior, bug, or error

type-crash

A hard crash of the interpreter, possibly with a core dump

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

ofey404 commented Oct 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ofey404 commented Oct 2, 2022

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Oct 2, 2022

Uh oh!

bedevere-bot commented Oct 2, 2022

Uh oh!

bedevere-bot commented Oct 2, 2022

Uh oh!

gvanrossum commented Oct 2, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ofey404 commented Oct 1, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.