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

test_import crashes with a --forever option #119584

Closed
Eclips4 opened this issue May 26, 2024 · 3 comments
Closed

test_import crashes with a --forever option #119584

Eclips4 opened this issue May 26, 2024 · 3 comments
Assignees
@ericsnowcurrently
Labels
tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@Eclips4
Copy link
Member

Eclips4 commented May 26, 2024 •
edited by bedevere-app bot
Loading

Crash report

Bug description:

./python.exe -m test -v test_import -m test_check_state_first --forever
== CPython 3.14.0a0 (heads/main:5d04cc50e5, May 26 2024, 21:33:31) [Clang 15.0.0 (clang-1500.3.9.4)]
== macOS-14.5-arm64-arm-64bit-Mach-O little-endian
== Python build: debug
== cwd: /Users/admin/Projects/cpython/build/test_python_worker_94626æ
== CPU count: 8
== encodings: locale=UTF-8 FS=utf-8
== resources: all test resources are disabled, use -u option to unskip tests

Using random seed: 4127756675
0:00:00 load avg: 4.71 Run tests sequentially
0:00:00 load avg: 4.71 [  1] test_import
test_check_state_first (test.test_import.SinglephaseInitTests.test_check_state_first) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.001s

OK
0:00:00 load avg: 4.71 [  2] test_import
test_check_state_first (test.test_import.SinglephaseInitTests.test_check_state_first) ... Assertion failed: (_testsinglephase_with_reinit_check_cache_first.m_base.m_index == 0), function PyInit__testsinglephase_with_reinit_check_cache_first, file _testsinglephase.c, line 714.
Fatal Python error: Aborted

Current thread 0x00000001f61d0c00 (most recent call first):
  File "<frozen importlib._bootstrap>", line 488 in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 1316 in create_module
  File "<frozen importlib._bootstrap>", line 813 in module_from_spec
  File "<frozen importlib._bootstrap>", line 921 in _load_unlocked
  File "<frozen importlib._bootstrap>", line 966 in _load
  File "/Users/admin/Projects/cpython/Lib/test/test_import/__init__.py", line 2495 in _load_dynamic
  File "/Users/admin/Projects/cpython/Lib/test/test_import/__init__.py", line 2894 in test_check_state_first
  File "/Users/admin/Projects/cpython/Lib/unittest/case.py", line 606 in _callTestMethod
  File "/Users/admin/Projects/cpython/Lib/unittest/case.py", line 651 in run
  File "/Users/admin/Projects/cpython/Lib/unittest/case.py", line 707 in __call__
  File "/Users/admin/Projects/cpython/Lib/unittest/suite.py", line 122 in run
  File "/Users/admin/Projects/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/Users/admin/Projects/cpython/Lib/unittest/suite.py", line 122 in run
  File "/Users/admin/Projects/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/Users/admin/Projects/cpython/Lib/unittest/runner.py", line 240 in run
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 57 in _run_suite
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 37 in run_unittest
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 135 in test_func
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 91 in regrtest_runner
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 138 in _load_run_test
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 181 in _runtest_env_changed_exc
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 281 in _runtest
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/single.py", line 310 in run_single_test
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/main.py", line 355 in run_test
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/main.py", line 389 in run_tests_sequentially
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/main.py", line 533 in _run_tests
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/main.py", line 568 in run_tests
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/main.py", line 731 in main
  File "/Users/admin/Projects/cpython/Lib/test/libregrtest/main.py", line 739 in main
  File "/Users/admin/Projects/cpython/Lib/test/__main__.py", line 2 in <module>
  File "/Users/admin/Projects/cpython/Lib/runpy.py", line 88 in _run_code
  File "/Users/admin/Projects/cpython/Lib/runpy.py", line 198 in _run_module_as_main

Extension modules: _testinternalcapi, _testmultiphase (total: 2)
zsh: abort      ./python.exe -m test -v test_import -m test_check_state_first --forever

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs
@Eclips4 Eclips4 added tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump labels May 26, 2024
@Eclips4
Copy link
Member Author

Eclips4 commented May 26, 2024

Bisected to 0c5ebe1
cc @ericsnowcurrently

@Eclips4
Copy link
Member Author

Eclips4 commented May 27, 2024

I'm guess this test (and two other tests which are similar) is cannot be run multiple times:

cpython/Modules/_testsinglephase.c

Lines 692 to 701 in 5482a93

PyMODINIT_FUNC
PyInit__testsinglephase_check_cache_first(void)
{
assert(_testsinglephase_check_cache_first.m_base.m_index == 0);
PyObject *mod = PyState_FindModule(&_testsinglephase_check_cache_first);
if (mod != NULL) {
return Py_NewRef(mod);
}
return PyModule_Create(&_testsinglephase_check_cache_first);
}

Firstly we check if m_index == 0, which means that our module is not in the cache. Then, we try to find this module using the PyState_FindModule which relies on the m_index field, and works only when m_index >= 0. These two steps seem weird for me.
1)We can just mark the test_check_state_first with a no_rerun decorator, due to the reason that this test is to check whether initializing the module with different m_size works.
2) Change the assert to m_index >= 0 instead of m_index == 0

@AlexWaygood AlexWaygood mentioned this issue May 27, 2024
Closed
@ericsnowcurrently ericsnowcurrently self-assigned this May 27, 2024
@ericsnowcurrently
Copy link
Member

I'll have a fix up shortly.

@bedevere-app bedevere-app bot mentioned this issue May 27, 2024
Merged
ericsnowcurrently added a commit that referenced this issue May 27, 2024
@ericsnowcurrently
gh-119584: Fix test_import Failed Assertion (gh-119623)
ae7b176 
The fix in gh-119561 introduced an assertion that doesn't hold true if any of the three new test extension modules are loaded more than once.  This is fine normally but breaks if the new test_check_state_first() is run more than once, which happens for refleak checking and with the regrtest --forever flag.  We fix that here by clearing each of the three modules after loading them.  We also tweak a check in _modules_by_index_check().
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 27, 2024
@ericsnowcurrently @miss-islington
pythongh-119584: Fix test_import Failed Assertion (pythongh-119623)
cf9c610 
The fix in pythongh-119561 introduced an assertion that doesn't hold true if any of the three new test extension modules are loaded more than once.  This is fine normally but breaks if the new test_check_state_first() is run more than once, which happens for refleak checking and with the regrtest --forever flag.  We fix that here by clearing each of the three modules after loading them.  We also tweak a check in _modules_by_index_check().
(cherry picked from commit ae7b176)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue May 27, 2024
Merged
ericsnowcurrently added a commit that referenced this issue May 27, 2024
@miss-islington @ericsnowcurrently
[3.13] gh-119584: Fix test_import Failed Assertion (gh-119623) (gh-11…
660125f 
…9633)

The fix in gh-119561 introduced an assertion that doesn't hold true if any of the three new test extension modules are loaded more than once.  This is fine normally but breaks if the new test_check_state_first() is run more than once, which happens for refleak checking and with the regrtest --forever flag.  We fix that here by clearing each of the three modules after loading them.  We also tweak a check in _modules_by_index_check().

(cherry picked from commit ae7b176)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@JelleZijlstra JelleZijlstra mentioned this issue May 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ericsnowcurrently ericsnowcurrently

Labels
tests Tests in the Lib/test dir 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
2 participants
@ericsnowcurrently @Eclips4