-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
gh-128942: make arraymodule.c free-thread safe (lock-free) #130771
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
Open
tom-pytel
wants to merge
65
commits into
python:main
Choose a base branch
from
tom-pytel:fix-issue-128942-lockfree
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,163
−361
Open
Changes from 1 commit
Commits
Show all changes
65 commits
Select commit
Hold shift + click to select a range
f54c5fc
slow arraymodule
tom-pytel 4c71f00
lockfree read and write single element
tom-pytel 512c4c7
📜🤖 Added by blurb_it.
blurb-it[bot] 060100f
ensure_shared_on_resize() in one more place
tom-pytel d0b17c6
fix stupid direct return out of critical section
tom-pytel c17b787
requested changes
tom-pytel 4fd8383
downgrade to _Py_atomic_load_ptr_relaxed in missed place
tom-pytel d00f2b7
arraymodule linked statically
tom-pytel a3e6004
cleanups
tom-pytel fb6212a
test and tsan stuff
tom-pytel 04a8f9d
getters and setters using atomics
tom-pytel 01423fd
fix 2 byte wchar_t
tom-pytel 99331dd
remove debug printf
tom-pytel 07c98cc
just a hunch...
tom-pytel cf3bbbb
atomic "memcpy"
tom-pytel 51135a4
misc
tom-pytel fff827e
use proper type FT_ macros
tom-pytel 12f0ff6
atomic aggregate _Py_atomic_source_memcpy_relaxed()
tom-pytel 1a6b8df
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 94ef417
remove atomic memcpy
tom-pytel 0056412
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 1431784
regen clinic
tom-pytel 460d3d7
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 409239c
requested changes
tom-pytel a6f17c9
more requested changes
tom-pytel b5d219e
__declspec(align(8)) for Windows
tom-pytel 2c82071
shut up check-c-globals
tom-pytel 1ba50e9
alignment changes
tom-pytel 576aebf
MS_WINDOWS -> _MSC_VER
tom-pytel 4dd0954
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel d4e5313
#include "pycore_gc.h", something moved
tom-pytel 48eabe3
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel c056b13
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel affae8e
remove NULL check in arraydata_free()
tom-pytel 689c7a3
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 3e2f8cd
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 7286fed
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 6550906
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 5b35203
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 15d92ca
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 2e7132e
change to use new _Py_ALIGN_AS() macro
tom-pytel 06f86cf
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel d29c208
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 8db665a
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel cad8ca2
Merge in the main branch, switch to _Py_ALIGNED_DEF
encukou 1a50981
requested changes
tom-pytel f023e77
shut up UBSan
tom-pytel b53ba2a
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel eb60ffa
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel 6d50e9a
add size to _PyMem_FreeDelayed()
tom-pytel 711d86e
Merge branch 'main' into fix-issue-128942-lockfree
tom-pytel a813353
fix for recent changes to Setup.stdlib.in
tom-pytel 684b953
fix broken fix of Setup.stdlib.in
tom-pytel 0cec45d
with open(), remove test_array from TSAN tests
tom-pytel fd97dc3
Update Include/internal/pycore_pymem.h
tom-pytel 229383b
Update Include/internal/pycore_pymem.h
tom-pytel 8198469
remove array module from Setup.stdlib.in
tom-pytel 8af6be7
requested changes
tom-pytel 201e7ad
rest of requested changes
tom-pytel 0bd7ec1
remove import os from test_array
tom-pytel b8bdb13
add assert size * itemisze < size_t in buffer alloc func
tom-pytel 05b1a91
fix formatting
tom-pytel 60f6e5b
fix and clean up array size checks everywhere
tom-pytel 30cfdbf
fix sizeof signed compare
tom-pytel 07c9c15
remove misc comment
tom-pytel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
requested changes
- Loading branch information
commit 1a509813d640d9c610facb0172479899c931bc54
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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.
Use
Py_ssize_t
for sizes.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.
In this case the
size_t
was in the original code and the signature isPyMem_Malloc(size_t size)
so it needs to be cast anyway... Your call, confirm yes or no.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.
To my knowledge, we typically use
Py_ssize_t
in most places forPyMem_Malloc
, and I'd say that it fits nicely here since thesize
parameter is already aPy_ssize_t
. Let's change it.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.
I saw guards like
if (size > PY_SSIZE_T_MAX / descr->itemsize)
. Maybe it is worth to add an assert here to be sure thatsize * itemsize
doesn't overflowsize_t
?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.
Yes there are checks everywhere, but also yes an assert here is not a bad idea.