GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-130209: Retry path-based os functions on EINTR by larryhastings · Pull Request #160544 · python/cpython

Conversation

@larryhastings

@larryhastings larryhastings commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Extends PEP 475's retry-on-EINTR behavior to more os functions that take a path (and a few fd-based stragglers). This means these functions can now succeed if a signal arrives during a slow filesystem operation (e.g. networked or user filesystems).

Background

PEP 475 (3.5) added EINTR retry loops to the functions expected to block: read, write, open, fstat, select, sockets, wait*, and so on. The path-based functions (stat, rename, utime, unlink, listdir, ...) were left out on the assumption that they don't block long enough to be interrupted. That assumption doesn't hold on network or userspace filesystems:

  • NFSv4 (Linux fs/nfs/nfs4proc.c): when the server answers NFS4ERR_DELAY/GRACE the client backs off in TASK_INTERRUPTIBLE for lookup, getattr, access, readdir, remove, mkdir, open and friends, and any signal makes the syscall return -ERESTARTSYS. Since signal.signal() installs handlers without SA_RESTART, that reaches Python as EINTR. Every path walk goes through lookup/access/getattr, so stat, rename, utime, unlink etc. are all exposed.
  • FUSE (fs/fuse/dev.c, request_wait_answer): a request still queued when a signal arrives fails with a hard -EINTR, which SA_RESTART cannot restart. sshfs, rclone, and any other FUSE filesystem can produce this. A userspace retry is the only fix.

The original report came from a program with a SIGCHLD handler doing routine bookkeeping on an NFS mount; every child exit was a chance for a path operation in the main thread to die with EINTR.

os.access() is a special case worth calling out: it reports failure by return value, so before this change an EINTR made it silently return False for a perfectly readable file, with no exception at all.

Changes

Modules/posixmodule.c only. Retry loops added to 30 C functions, covering these Python-level functions:

os.access, os.chdir, os.chflags, os.chmod, os.chown, os.chroot, os.getxattr, os.lchflags, os.lchmod, os.lchown, os.link, os.listdir, os.listxattr, os.lstat, os.mkdir, os.readlink, os.removexattr, os.rename, os.replace, os.rmdir, os.scandir (open, iteration, and DirEntry.stat()/is_*()), os.setxattr, os.stat, os.statvfs, os.statx, os.symlink, os.truncate, os.unlink, os.utime, and os._fcopyfile (macOS).

Behavior on retry matches PEP 475 exactly: on EINTR, run the Python signal handlers via PyErr_CheckSignals(); if none raises, call again; if one raises, propagate that exception. Error reporting is unchanged (path_error, path_error2, etc. are called exactly as before). Windows builds compile the loops but EINTR never occurs there.

The code is mostly mechanical cookie-cutter code as follows, adjusted to conform to function-specific details:

for (;;) {
    Py_BEGIN_ALLOW_THREADS
    result = syscall(...);
    Py_END_ALLOW_THREADS

    // interrupted by a signal (EINTR)?  retry!
    if ((result != 0) && (errno == EINTR)) {
        if (PyErr_CheckSignals()) {
            return NULL;
        }
        continue;
    }
    break;
}

Two pre-existing bugs fixed in passing, in code that had to be restructured anyway:

  • os.scandir(fd) on a platform without fdopendir leaked the half-built iterator object when raising TypeError.
  • DirEntry.stat() carried an errno save/restore around Py_DECREF that became dead once the error is reported before the decref; removed.

Testing

  • Full test suite passes on Linux.
  • Every changed function was run through the preprocessor under Linux, macOS, bare-POSIX (no *at functions), and Windows macro sets to verify {} and Py_BEGIN/END_ALLOW_THREADS balance in each configuration.
  • A FUSE test harness exercises 15 of the functions, two cases each: handler returns normally → call succeeds and handler ran exactly once; handler raises → that exception propagates. All 30 cases pass with this branch. On an unmodified interpreter, 28 report InterruptedError and the two os.access cases return False for a readable file. (I could post the FUSE test harness if there was interest; it's unavoidably Linux-only.)

Not compiled/tested locally: os_lchmod_impl, os_chflags_impl, os_lchflags_impl, os__fcopyfile_impl. I can't even compile those on my Linux box. Reviewers may want to keep an eye on the macOS job for those.

AI Involvement

I wrote the code, Claude Fable 5.1 wrote the tests and I reviewed them.

Issue: gh-130209

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@larryhastings
Extends PEP 475 retry behavior to more os functions that take a path.
f867952
PEP 475 (3.5) added "EINTR" retry loops to the functions expected to block:
read, write, open, stat, select, sockets, wait*, and so on.  The path-based
functions (stat, rename, utime, unlink, listdir, ...) were skipped,
on the assumption that they don't block long enough to be interrupted.

That assumption doesn't hold on network or userspace filesystems.
This commit adds retry loops to thirty more functions, to fill in the gap:

    * os.access
    * os.chdir
    * os.chflags
    * os.chmod
    * os.chown
    * os.chroot
    * os.getxattr
    * os.lchflags
    * os.lchmod
    * os.lchown
    * os.link
    * os.listdir
    * os.listxattr
    * os.lstat
    * os.mkdir
    * os.readlink
    * os.removexattr
    * os.rename
    * os.replace
    * os.rmdir
    * os.scandir (open, iteration, and DirEntry.stat() / is_*() )
    * os.setxattr
    * os.stat
    * os.statvfs
    * os.statx
    * os.symlink
    * os.truncate
    * os.unlink
    * os.utime
    * os._fcopyfile

os.access() is a special case worth calling out: it reports failure by
return value, so before this change an EINTR made it silently return
False for a perfectly readable file--with no exception at all.

Issue: pythongh-130209
@bedevere-app bedevere-app Bot added awaiting core review type-feature A feature request or enhancement labels Oct 23, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Oct 23, 2026
Make os.scandir retry on system calls failing with EINTR #130209
Open
@larryhastings
larryhastings requested review from encukou, picnixz and vstinner October 23, 2026 23:37
@larryhastings larryhastings self-assigned this Oct 23, 2026
@larryhastings
Merge branch 'main' into pep475plus
a971275
@larryhastings larryhastings added type-bug An unexpected behavior, bug, or error OS-mac extension-modules C modules in the Modules dir OS-linux OS-netbsd and removed type-feature A feature request or enhancement labels Oct 24, 2026
@encukou

encukou commented Oct 30, 2026

Copy link
Copy Markdown
Member

(I'll review next week. Sorry I didn't get to it; I promised too much to people at the sprint.)

larryhastings reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

encukou
encukou reviewed Nov 3, 2026
View reviewed changes

@encukou encukou left a comment

Copy link
Copy Markdown
Member

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.

The main issue I see is handling non-idempotent operations (like mkdir, rmdir, rename, readdir, unlink, symlink, fcopyfile, chdir, chroot, removexattr; setattr with XATTR_CREATE).
I checked my system's man pages for a few of them; all I saw don't mention EINTR handling (unlike for example write(2) which says “The call was interrupted by a signal before any data was written”). I'm not sure if we can always assume that the FS operation did not happen yet. Do you know if that's documented/standardized somewhere?
NFS/FUSE sounds like the wild West in terms of details like this; are we regressing on some systems?


Can you add the tests? We already test some stuff that needs root.


Otherwise, I have a few comments, and nits (ignore those if your choices are deliberate).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Misc/NEWS.d/next/Library/2026-10-23-16-06-39.gh-issue-130209.q_hzoM.rst
Comment on lines +1 to +10
Added retry loops to functions in :mod:`os` that interact with the
filesystem, such as :func:`~os.stat`, :func:`~os.rename`, :func:`~os.utime`,
:func:`~os.unlink`, :func:`~os.listdir` and :func:`~os.scandir`. On POSIX,
if a system call fails and ``errno`` is set to ``EINTR``, it means a signal
was delivered to the process and interrupted the call; the recommended
course of action is to handle the signal and retry. :pep:`475` added retry
loops for a number of functions in the :mod:`os` module, but skipped most of
the functions that take a *path* argument, because those don't normally
result in ``EINTR``. However, they *can* (and do!) on network filesystems
(e.g. NFS) and userspace filesystems (e.g. FUSE).

@encukou encukou Nov 2, 2026

Copy link
Copy Markdown
Member

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.

This seems too long for a news entry; consider:

Suggested change
Added retry loops to functions in :mod:`os` that interact with the
filesystem, such as :func:`~os.stat`, :func:`~os.rename`, :func:`~os.utime`,
:func:`~os.unlink`, :func:`~os.listdir` and :func:`~os.scandir`. On POSIX,
if a system call fails and ``errno`` is set to ``EINTR``, it means a signal
was delivered to the process and interrupted the call; the recommended
course of action is to handle the signal and retry. :pep:`475` added retry
loops for a number of functions in the :mod:`os` module, but skipped most of
the functions that take a *path* argument, because those don't normally
result in ``EINTR``. However, they *can* (and do!) on network filesystems
(e.g. NFS) and userspace filesystems (e.g. FUSE).
Added retry loops to functions in :mod:`os` that interact with the
filesystem, such as :func:`~os.stat`, :func:`~os.rename`, :func:`~os.utime`,
:func:`~os.unlink`, :func:`~os.listdir` and :func:`~os.scandir`, which
:pep:`475` skipped because those don't normally
result in ``EINTR``. They *can* (and do!) on network filesystems
(e.g. NFS) and userspace filesystems (e.g. FUSE).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
#endif
result = rename(src->narrow, dst->narrow);
Py_END_ALLOW_THREADS
result = rename(src->narrow, dst->narrow);

@encukou encukou Nov 2, 2026

Copy link
Copy Markdown
Member

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.

Nit: This line might be after else; unlike other such cases isn't indented, so it's more confusing/misleading than usual. Please indent it; ideally add braces too.

Suggested change
result = rename(src->narrow, dst->narrow);
{
result = rename(src->narrow, dst->narrow);
}

(same in utime_default below)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
PyErr_SetString(PyExc_NotImplementedError, "can't fetch stat");
return NULL;
}
} else

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

The existing code uses PEP-7 style; please don't change it.

Suggested change
} else
}
else

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
Py_END_ALLOW_THREADS

// interrupted by a signal (EINTR)? retry!
if ((result != 0) && (errno == EINTR)) {

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

Nit: Result checking is understandably inconsistent between functions. But within functions, consider making it textually identical -- either:

Suggested change
if ((result != 0) && (errno == EINTR)) {
if ((result) && (errno == EINTR)) {

or change to if (result != 0) below.

Comparing (result) to (result != 0) takes a bit cognitive effort that's better spent elsewhere :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
Py_END_ALLOW_THREADS

// interrupted by a signal (EINTR)? retry!
if ((result != 0) && (errno == EINTR)) {

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

Please use the same condition within a function.

Suggested change
if ((result != 0) && (errno == EINTR)) {
if ((result < 0) && (errno == EINTR)) {

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
Comment on lines +6910 to +6922
Py_BEGIN_ALLOW_THREADS
#ifdef HAVE_UNLINKAT
if (dir_fd != DEFAULT_DIR_FD) {
if (HAVE_UNLINKAT_RUNTIME) {

result = unlinkat(dir_fd, path->narrow, 0);
} else {
unlinkat_unavailable = 1;
}
} else
if (dir_fd != DEFAULT_DIR_FD) {
if (HAVE_UNLINKAT_RUNTIME) {
result = unlinkat(dir_fd, path->narrow, 0);
} else {
unlinkat_unavailable = 1;
result = 0;
}
} else
#endif /* HAVE_UNLINKAT */
result = unlink(path->narrow);
result = unlink(path->narrow);
Py_END_ALLOW_THREADS

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

Nit: Moving the Py_[BEGIN|END]_ALLOW_THREADS further inwards would allow handling !HAVE_UNLINKAT_RUNTIME here; you could remove the unlinkat_unavailable variable & artificial result:

#ifdef HAVE_UNLINKAT
        if (dir_fd != DEFAULT_DIR_FD) {
          if (HAVE_UNLINKAT_RUNTIME) {
            Py_BEGIN_ALLOW_THREADS
            result = unlinkat(dir_fd, path->narrow, 0);
            Py_END_ALLOW_THREADS
          } else {
            argument_unavailable_error(NULL, "dir_fd");
            return NULL;
          }
        } else
#endif /* HAVE_UNLINKAT */
        {
            Py_BEGIN_ALLOW_THREADS
            result = unlink(path->narrow);
            Py_END_ALLOW_THREADS
        }

(There's a bunch of cases like this in other functions.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
Py_END_ALLOW_THREADS

// interrupted by a signal (EINTR)? retry!
if ((result != 0) && (errno == EINTR)) {

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

Please use the same condition.

Suggested change
if ((result != 0) && (errno == EINTR)) {
if ((result < 0) && (errno == EINTR)) {

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
Py_BEGIN_ALLOW_THREADS
if (path->is_fd) {
result = fstatfs(path->fd, &st);
for (;;) {

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

nit: os.statvfs would now be much shorter with 3 individual #ifdef __APPLE__ blocks for the differences.
I'm not entirely sure if it's more readable that way, though.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c

// interrupted by a signal (EINTR)? retry!
if ((direntp == NULL) && (error == EINTR)) {
if (!PyErr_CheckSignals()) {

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

PyErr_CheckSignals can run user code. It would be better to unlock &iterator->read_mutex around it to avoid a deadlock.

(Hopefully no one reads directories in a signal handler, but better be safe.)

AFAICS, that means means the outer while can do the for(;;)'s job.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Modules/posixmodule.c
Comment on lines +17604 to +17605
// suppress error--this means we don't run path_error
// below, so we'll raise the existing already-set exception

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

That's not the only thing that happens, though. There's also a call to ScandirIterator_closedir, which I don't think we want for errors from a signal handler.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
encukou
encukou reviewed Nov 3, 2026
View reviewed changes
Comment thread Modules/posixmodule.c
// interrupted by a signal (EINTR)? retry!
if ((dirp == NULL) && (errno == EINTR)) {
if (PyErr_CheckSignals()) {
close(fd);

@encukou encukou Nov 3, 2026

Copy link
Copy Markdown
Member

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.

close can presumably block on NTFS, so it should go in Py_BEGIN_ALLOW_THREADS.

(And these should technically get their own EINTR handling, but I assume a FD leak here isn't worth the complexity.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments
@vstinner vstinner Awaiting requested review from vstinner
@picnixz picnixz Awaiting requested review from picnixz

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@larryhastings larryhastings

Labels

awaiting core review extension-modules C modules in the Modules dir OS-linux OS-mac OS-netbsd type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@larryhastings @encukou
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.