Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Free Threading] test_opcache: test_binary_subscr_list_int(): list_get_item_ref: Assertion `cap != -1 && cap >= size' failed #127521

Closed
vstinner opened this issue Dec 2, 2024 · 2 comments
Closed

[Free Threading] test_opcache: test_binary_subscr_list_int(): list_get_item_ref: Assertion `cap != -1 && cap >= size' failed #127521

vstinner opened this issue Dec 2, 2024 · 2 comments
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@vstinner
Copy link
Member

vstinner commented Dec 2, 2024 •
edited by bedevere-app bot
Loading

test_binary_subscr_list_int() does crash randomly with an assertion error if Python is built with --disable-gil:

$ ./python -m test test_opcache -v -m test_binary_subscr_list_int -j10 -F
(...)
test_binary_subscr_list_int (test.test_opcache.TestRacesDoNotCrash.test_binary_subscr_list_int) ...

python: Objects/listobject.c:342: list_get_item_ref: Assertion `cap != -1 && cap >= size' failed.
Fatal Python error: Aborted

Thread 0x00007f244350f6c0 (most recent call first):
  File "/home/vstinner/python/main/Lib/test/test_opcache.py", line 638 in write
  File "/home/vstinner/python/main/Lib/threading.py", line 992 in run
  File "/home/vstinner/python/main/Lib/threading.py", line 1041 in _bootstrap_inner
  File "/home/vstinner/python/main/Lib/threading.py", line 1012 in _bootstrap

Thread 0x00007f2443d106c0 (most recent call first):
  File "/home/vstinner/python/main/Lib/test/test_opcache.py", line 639 in write
  File "/home/vstinner/python/main/Lib/threading.py", line 992 in run
  File "/home/vstinner/python/main/Lib/threading.py", line 1041 in _bootstrap_inner
  File "/home/vstinner/python/main/Lib/threading.py", line 1012 in _bootstrap

Current thread 0x00007f2451be5740 (most recent call first):
  File "/home/vstinner/python/main/Lib/test/test_opcache.py", line 632 in read
  File "/home/vstinner/python/main/Lib/test/test_opcache.py", line 586 in assert_races_do_not_crash
  File "/home/vstinner/python/main/Lib/test/test_opcache.py", line 22 in wrapper
  File "/home/vstinner/python/main/Lib/test/test_opcache.py", line 642 in test_binary_subscr_list_int
  (...)

Extension modules: _testinternalcapi (total: 1)

Bug seen on macOS CI, and reproduced on Linux.

Linked PRs
@vstinner vstinner mentioned this issue Dec 2, 2024
Merged
@vstinner
Copy link
Member Author

vstinner commented Dec 2, 2024

cc @corona10 @ZeroIntensity

According to git bisect, the regression was introduced by commit e271340.

commit e2713409cff5b71b1176b0e3fa63dae447548672
Author: Donghee Na <donghee.na@python.org>
Date:   Mon Dec 2 10:38:17 2024 +0900

    gh-115999: Add partial free-thread specialization for BINARY_SUBSCR (gh-127227)

@vstinner vstinner mentioned this issue Dec 2, 2024
Merged
@colesbury
Copy link
Contributor

I think I understand the issue. It's a preexisting bug in the free threading listobject.c implementation. We need to be setting the _PyObject_GC_IS_SHARED flag when resizing if the current thread doesn't own the list.

colesbury added a commit to colesbury/cpython that referenced this issue Dec 2, 2024
@colesbury
pythongh-127521: Mark list as "shared" before resizing if necessary
810d483 
In the free threading build, f a non-owning thread resizes a list,
it must use QSBR to free the old list array because there may be a
concurrent access (without a lock) from the owning thread.

To match the pattern in dictobject.c, we just mark the list as "shared"
before resizing if it's from a non-owning thread and not already marked
as shared.
@bedevere-app bedevere-app bot mentioned this issue Dec 2, 2024
Merged
colesbury added a commit that referenced this issue Dec 2, 2024
@colesbury
gh-127521: Mark list as "shared" before resizing if necessary (#127524)
c7dec02 
In the free threading build, if a non-owning thread resizes a list,
it must use QSBR to free the old list array because there may be a
concurrent access (without a lock) from the owning thread.

To match the pattern in dictobject.c, we just mark the list as "shared"
before resizing if it's from a non-owning thread and not already marked
as shared.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 2, 2024
@colesbury @miss-islington
pythongh-127521: Mark list as "shared" before resizing if necessary (p…
e46b077 
…ythonGH-127524)

In the free threading build, if a non-owning thread resizes a list,
it must use QSBR to free the old list array because there may be a
concurrent access (without a lock) from the owning thread.

To match the pattern in dictobject.c, we just mark the list as "shared"
before resizing if it's from a non-owning thread and not already marked
as shared.
(cherry picked from commit c7dec02)

Co-authored-by: Sam Gross <colesbury@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue Dec 2, 2024
Merged
@picnixz picnixz added type-crash A hard crash of the interpreter, possibly with a core dump interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Dec 2, 2024
colesbury added a commit that referenced this issue Dec 3, 2024
@miss-islington @colesbury
[3.13] gh-127521: Mark list as "shared" before resizing if necessary (G…
e546876 
…H-127524) (GH-127533)

In the free threading build, if a non-owning thread resizes a list,
it must use QSBR to free the old list array because there may be a
concurrent access (without a lock) from the owning thread.

To match the pattern in dictobject.c, we just mark the list as "shared"
before resizing if it's from a non-owning thread and not already marked
as shared.
(cherry picked from commit c7dec02)

Co-authored-by: Sam Gross <colesbury@gmail.com>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this issue Jan 8, 2025
@colesbury @srinivasreddy
pythongh-127521: Mark list as "shared" before resizing if necessary (p…
cf96e28 
…ython#127524)

In the free threading build, if a non-owning thread resizes a list,
it must use QSBR to free the old list array because there may be a
concurrent access (without a lock) from the owning thread.

To match the pattern in dictobject.c, we just mark the list as "shared"
before resizing if it's from a non-owning thread and not already marked
as shared.
ebonnal pushed a commit to ebonnal/cpython that referenced this issue Jan 12, 2025
@colesbury @ebonnal
pythongh-127521: Mark list as "shared" before resizing if necessary (p…
7be3aca 
…ython#127524)

In the free threading build, if a non-owning thread resizes a list,
it must use QSBR to free the old list array because there may be a
concurrent access (without a lock) from the owning thread.

To match the pattern in dictobject.c, we just mark the list as "shared"
before resizing if it's from a non-owning thread and not already marked
as shared.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vstinner @colesbury @picnixz