Skip to content

gh-109613: _pystat_fromstructstat() checks for exceptions #109618

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

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Sep 20, 2023

Fix os.stat() and os.DirEntry.stat(): check for exceptions. Previously, on Python built in debug mode, these functions could trigger a fatal Python error (and abort the process) when a function succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is raised, rather only checking for exceptions at the end. It fix following fatal error in fill_time():

Fatal Python error: _Py_CheckSlotResult:
Slot * of type int succeeded with an exception set

Fix os.stat() and os.DirEntry.stat(): check for exceptions.
Previously, on Python built in debug mode, these functions could
trigger a fatal Python error (and abort the process) when a function
succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is
raised, rather only checking for exceptions at the end. It fix
following fatal error in fill_time():

    Fatal Python error: _Py_CheckSlotResult:
    Slot * of type int succeeded with an exception set
@vstinner
Copy link
Member Author

On the main branch, the bug can easily be reproduced with these two scripts:

With this fix, I can no longer reproduce the bug.

vstinner@mona$ ./python repro_thread.py 
vstinner@mona$ ./python repro_thread.py 
vstinner@mona$ ./python repro_thread.py 
vstinner@mona$ ./python repro_thread.py 
vstinner@mona$ ./python repro_thread.py 

=> no crash

And:

vstinner@mona$ ./python stress_fill_time.py 
Stress test for 10 seconds...
/home/vstinner/python/main/stress_fill_time.py:25: ResourceWarning: unclosed scandir iterator <posix.ScandirIterator object at 0x7fe7cd4999c0>
Got 1000 SIGALRM signals in 10.0 seconds

vstinner@mona$ ./python stress_fill_time.py 
Stress test for 10 seconds...
<frozen importlib._bootstrap_external>:1199: ResourceWarning: unclosed file <_io.BufferedReader name='/home/vstinner/python/main/Lib/re/__pycache__/_casefix.cpython-313.pyc'>
Exception ignored in: <function _WeakValueDictionary.__init__.<locals>.KeyedRef.remove at 0x7fcdb03619d0>
Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 86, in remove
  File "/home/vstinner/python/main/stress_fill_time.py", line 16, in sig_alarm
    raise GotSignal()
GotSignal: 
/home/vstinner/python/main/stress_fill_time.py:25: ResourceWarning: unclosed scandir iterator <posix.ScandirIterator object at 0x7fcda2b999c0>
Got 1000 SIGALRM signals in 10.0 seconds

vstinner@mona$ ./python stress_fill_time.py 
Stress test for 10 seconds...
/home/vstinner/python/main/stress_fill_time.py:25: ResourceWarning: unclosed scandir iterator <posix.ScandirIterator object at 0x7f69f419d9c0>
Got 1000 SIGALRM signals in 10.0 seconds

vstinner@mona$ ./python stress_fill_time.py 
Stress test for 10 seconds...
/home/vstinner/python/main/stress_fill_time.py:25: ResourceWarning: unclosed scandir iterator <posix.ScandirIterator object at 0x7f20e079d9c0>
Got 1000 SIGALRM signals in 10.0 seconds

=> no crash. ResourceWarning and Exception ignored in: <function _WeakValueDictionary.__init__.<locals>.KeyedRef.remove...> are unrelated.

@vstinner
Copy link
Member Author

@chgnrdv: Would you mind to validate manually my fix?

@chgnrdv
Copy link
Contributor

chgnrdv commented Sep 20, 2023

@vstinner, I confirm that I can't reproduce the issue with both repros if your fix is applied. Code changes look good to me as well.

@vstinner
Copy link
Member Author

Thanks for testing and reviewing my change!

@vstinner vstinner merged commit d4cea79 into python:main Sep 21, 2023
@vstinner vstinner deleted the fromstructstat branch September 21, 2023 07:55
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker d4cea794a7b9b745817d2bd982d35412aef04710 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2023
…onGH-109618)

Fix os.stat() and os.DirEntry.stat(): check for exceptions.
Previously, on Python built in debug mode, these functions could
trigger a fatal Python error (and abort the process) when a function
succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is
raised, rather only checking for exceptions at the end. It fix
following fatal error in fill_time():

    Fatal Python error: _Py_CheckSlotResult:
    Slot * of type int succeeded with an exception set
(cherry picked from commit d4cea79)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Sep 21, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 21, 2023
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora Stable 3.x has failed when building commit d4cea79.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/90/builds/3802) and take a look at the build logs.
  4. Check if the failure is related to this commit (d4cea79) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/90/builds/3802

Failed tests:

  • test_eintr

Failed subtests:

  • test_all - test.test_eintr.EINTRTests.test_all
  • test_lockf - main.FNTLEINTRTest.test_lockf

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le/build/Lib/test/_test_eintr.py", line 523, in test_lockf
    self._lock(fcntl.lockf, "lockf")
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le/build/Lib/test/_test_eintr.py", line 508, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.3 sec


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le/build/Lib/test/test_eintr.py", line 17, in test_all
    script_helper.run_test_script(script)
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le/build/Lib/test/support/script_helper.py", line 300, in run_test_script
    raise AssertionError(f"{name} failed")
AssertionError: script _test_eintr.py failed

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86-64 macOS 3.x has failed when building commit d4cea79.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/366/builds/5417) and take a look at the build logs.
  4. Check if the failure is related to this commit (d4cea79) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/366/builds/5417

Failed tests:

  • test.test_multiprocessing_forkserver.test_manager

Summary of the results of the build (if available):

==

Click to see traceback logs
remote: Enumerating objects: 16, done.        
remote: Counting objects:   6% (1/16)        
remote: Counting objects:  12% (2/16)        
remote: Counting objects:  18% (3/16)        
remote: Counting objects:  25% (4/16)        
remote: Counting objects:  31% (5/16)        
remote: Counting objects:  37% (6/16)        
remote: Counting objects:  43% (7/16)        
remote: Counting objects:  50% (8/16)        
remote: Counting objects:  56% (9/16)        
remote: Counting objects:  62% (10/16)        
remote: Counting objects:  68% (11/16)        
remote: Counting objects:  75% (12/16)        
remote: Counting objects:  81% (13/16)        
remote: Counting objects:  87% (14/16)        
remote: Counting objects:  93% (15/16)        
remote: Counting objects: 100% (16/16)        
remote: Counting objects: 100% (16/16), done.        
remote: Compressing objects:  11% (1/9)        
remote: Compressing objects:  22% (2/9)        
remote: Compressing objects:  33% (3/9)        
remote: Compressing objects:  44% (4/9)        
remote: Compressing objects:  55% (5/9)        
remote: Compressing objects:  66% (6/9)        
remote: Compressing objects:  77% (7/9)        
remote: Compressing objects:  88% (8/9)        
remote: Compressing objects: 100% (9/9)        
remote: Compressing objects: 100% (9/9), done.        
remote: Total 9 (delta 7), reused 2 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to 'd4cea794a7b9b745817d2bd982d35412aef04710'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at d4cea794a7 gh-109613: _pystat_fromstructstat() checks for exceptions (#109618)
Switched to and reset branch 'main'

configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libpython3.13d.a(dynamic_annotations.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libpython3.13d.a(pymath.o) has no symbols
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
./Modules/readline.c:1256:21: warning: incompatible function pointer types assigning to 'Function *' (aka 'int (*)(const char *, int)') from 'int (void)' [-Wincompatible-function-pointer-types]
    rl_startup_hook = on_startup_hook;
                    ^ ~~~~~~~~~~~~~~~
./Modules/readline.c:1258:23: warning: incompatible function pointer types assigning to 'Function *' (aka 'int (*)(const char *, int)') from 'int (void)' [-Wincompatible-function-pointer-types]
    rl_pre_input_hook = on_pre_input_hook;
                      ^ ~~~~~~~~~~~~~~~~~
2 warnings generated.
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups
ld: warning: -undefined dynamic_lookup may not work with chained fixups

make: *** [buildbottest] Error 5

@bedevere-app
Copy link

bedevere-app bot commented Sep 21, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Sep 21, 2023
vstinner added a commit that referenced this pull request Sep 21, 2023
…9618) (#109668)

gh-109613: _pystat_fromstructstat() checks for exceptions (#109618)

Fix os.stat() and os.DirEntry.stat(): check for exceptions.
Previously, on Python built in debug mode, these functions could
trigger a fatal Python error (and abort the process) when a function
succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is
raised, rather only checking for exceptions at the end. It fix
following fatal error in fill_time():

    Fatal Python error: _Py_CheckSlotResult:
    Slot * of type int succeeded with an exception set

(cherry picked from commit d4cea79)
csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
…on#109618)

Fix os.stat() and os.DirEntry.stat(): check for exceptions.
Previously, on Python built in debug mode, these functions could
trigger a fatal Python error (and abort the process) when a function
succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is
raised, rather only checking for exceptions at the end. It fix
following fatal error in fill_time():

    Fatal Python error: _Py_CheckSlotResult:
    Slot * of type int succeeded with an exception set
Yhg1s pushed a commit that referenced this pull request Oct 2, 2023
…109618) (#109641)

gh-109613: _pystat_fromstructstat() checks for exceptions (GH-109618)

Fix os.stat() and os.DirEntry.stat(): check for exceptions.
Previously, on Python built in debug mode, these functions could
trigger a fatal Python error (and abort the process) when a function
succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is
raised, rather only checking for exceptions at the end. It fix
following fatal error in fill_time():

    Fatal Python error: _Py_CheckSlotResult:
    Slot * of type int succeeded with an exception set
(cherry picked from commit d4cea79)

Co-authored-by: Victor Stinner <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…on#109618)

Fix os.stat() and os.DirEntry.stat(): check for exceptions.
Previously, on Python built in debug mode, these functions could
trigger a fatal Python error (and abort the process) when a function
succeeded with an exception set.

_pystat_fromstructstat() now exits immediately if an exception is
raised, rather only checking for exceptions at the end. It fix
following fatal error in fill_time():

    Fatal Python error: _Py_CheckSlotResult:
    Slot * of type int succeeded with an exception set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants