Skip to content

gh-120198: Fix crash when two thread edit __class__ - #120195

Merged
zooba merged 12 commits into
python:mainfrom
Fidget-Spinner:hello
Jun 11, 2024
Merged

gh-120198: Fix crash when two thread edit __class__#120195
zooba merged 12 commits into
python:mainfrom
Fidget-Spinner:hello

Conversation

@Fidget-Spinner

@Fidget-SpinnerFidget-Spinner commented Jun 7, 2024•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-appbedevere-appBot added tests Tests in the Lib/test dir awaiting core review labels Jun 7, 2024
@vstinner
vstinner marked this pull request as draft June 7, 2024 07:11
@vstinner

vstinner commented Jun 7, 2024

Copy link
Copy Markdown
Member

[FOR CI, DO NOT MERGE]

I marked your PR as a draft in this case.

@Fidget-SpinnerFidget-Spinner mentioned this pull request Jun 7, 2024
Closed
@Fidget-SpinnerFidget-Spinner changed the title [FOR CI, DO NOT MERGE]gh-120198: Fix crash from concurrent setting of __class__Jun 7, 2024
@Fidget-SpinnerFidget-Spinner changed the title gh-120198: Fix crash from concurrent setting of __class__[FOR TESTING ONLY] crasherJun 7, 2024
@Fidget-Spinner@Zheaoli
fix bug
02a5587
Co-Authored-By: Nadeshiko Manju <me@manjusaka.me>
@Fidget-SpinnerFidget-Spinner changed the title [FOR TESTING ONLY] crashergh-120198: Fix crash when two thread edit __class__Jun 10, 2024
@Fidget-Spinner
Fidget-Spinner marked this pull request as ready for review June 10, 2024 15:07
zooba
zooba reviewed Jun 10, 2024
View reviewed changes
Comment threadLib/test/test_free_threading/test_type.py Outdated
@zoobazooba added the needs backport to 3.12 only security fixes label Jun 10, 2024
@Fidget-Spinner@zooba
Move test like Steve suggested
1fc5c01
Co-Authored-By: Steve Dower <steve.dower@microsoft.com>
Zheaoli
Zheaoli approved these changes Jun 10, 2024
View reviewed changes

@ZheaoliZheaoli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

BTW should we backport this to 3.11?

@zooba

zooba commented Jun 10, 2024

Copy link
Copy Markdown
Member

If we go to 3.11 then we go to 3.8 (unless the audit event was added later). Technically we should, because crashers are technically eligible for security fix releases, but this one really requires deliberate exploitation and it's very unlikely to be achievable without arbitrary code. And the test is likely to destabilise earlier releases more than the fix is worth.

If someone wants to prepare a PR against 3.11 without the test, we can let the RMs decide.

@zooba
zooba merged commit 203565b into python:mainJun 11, 2024
@miss-islington-app

miss-islington-appBot commented Jun 11, 2024

Copy link
Copy Markdown

Thanks @Fidget-Spinner for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 11, 2024
@Fidget-Spinner@miss-islington
pythongh-120198: Fix race condition when editing __class__ with an au…
2a5692a
…dit hook active (pythonGH-120195)
(cherry picked from commit 203565b)
Co-authored-by: Ken Jin <kenjin@python.org>
@miss-islington-app

miss-islington-appBot commented Jun 11, 2024

Copy link
Copy Markdown

Sorry, @Fidget-Spinner and @zooba, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 203565b2f9c74656ba519780049b46d4e5afcba1 3.12

@bedevere-app

bedevere-appBot commented Jun 11, 2024

Copy link
Copy Markdown

GH-120366 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Jun 11, 2024
@zoobazooba assigned Fidget-Spinner and unassigned zoobaJun 11, 2024
@zooba

zooba commented Jun 11, 2024

Copy link
Copy Markdown
Member

@Fidget-Spinner Could you handle the backport, please?

zooba pushed a commit that referenced this pull request Jun 11, 2024
@miss-islington@Fidget-Spinner
gh-120198: Fix race condition when editing __class__ with an audit ho…
a6c4080
…ok active (GH-120195)
(cherry picked from commit 203565b)
Co-authored-by: Ken Jin <kenjin@python.org>
@Fidget-Spinner

Fidget-Spinner commented Jun 12, 2024

Copy link
Copy Markdown
MemberAuthor

@zooba sure!

@Fidget-Spinner

Fidget-Spinner commented Jun 12, 2024

Copy link
Copy Markdown
MemberAuthor

@Zheaoli so sorry I had planned to add you as co-author when merging but was asleep when this merged.

@Fidget-SpinnerFidget-Spinner mentioned this pull request Jun 12, 2024
Merged
@bedevere-app

bedevere-appBot commented Jun 12, 2024

Copy link
Copy Markdown

GH-120393 is a backport of this pull request to the 3.12 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.12 only security fixes label Jun 12, 2024
@Fidget-Spinner
Fidget-Spinner deleted the hello branch June 12, 2024 11:32
zooba pushed a commit that referenced this pull request Jun 12, 2024
@Fidget-Spinner@Zheaoli
gh-120198: Fix race condition when editing __class__ with an audit ho…
e8a9f0c
…ok active (GH-120195)
Co-authored-by: Nadeshiko Manju <me@manjusaka.me>
@Zheaoli

Zheaoli commented Jun 12, 2024

Copy link
Copy Markdown
Contributor

@Fidget-Spinner No problem sir! Thanks very much. I'm so happy to see we solve this issue

mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024
@Fidget-Spinner@mrahtz
pythongh-120198: Fix race condition when editing __class__ with an au…
a9a5889
…dit hook active (pythonGH-120195)
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
@Fidget-Spinner@noahbkim
pythongh-120198: Fix race condition when editing __class__ with an au…
7a06f46
…dit hook active (pythonGH-120195)
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@Fidget-Spinner@estyxx
pythongh-120198: Fix race condition when editing __class__ with an au…
7346d61
…dit hook active (pythonGH-120195)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner
@serhiy-storchakaserhiy-storchakaAwaiting requested review from serhiy-storchaka
@zoobazoobaAwaiting requested review from zooba
1 more reviewer
@ZheaoliZheaoliZheaoli approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@Fidget-SpinnerFidget-Spinner

Labels

testsTests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Fidget-Spinner@vstinner@zooba@Zheaoli