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.

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

gh-90102: Remove isatty call during regular open #124922

Merged
merged 10 commits into from
Oct 8, 2024
1 change: 1 addition & 0 deletions Include/internal/pycore_global_objects_fini_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Include/internal/pycore_global_strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ struct _Py_global_strings {
STRUCT_FOR_ID(_initializing)
STRUCT_FOR_ID(_io)
STRUCT_FOR_ID(_is_text_encoding)
STRUCT_FOR_ID(_isatty_open_only)
STRUCT_FOR_ID(_length_)
STRUCT_FOR_ID(_limbo)
STRUCT_FOR_ID(_lock_unlock_module)
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_runtime_init_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Include/internal/pycore_unicodeobject_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 16 additions & 1 deletion Lib/_pyio.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def open(file, mode="r", buffering=-1, encoding=None, errors=None,
result = raw
try:
line_buffering = False
if buffering == 1 or buffering < 0 and raw.isatty():
if buffering == 1 or buffering < 0 and raw._isatty_open_only():
buffering = -1
line_buffering = True
if buffering < 0:
Expand Down Expand Up @@ -1794,6 +1794,21 @@ def isatty(self):
self._checkClosed()
return os.isatty(self._fd)

def _isatty_open_only(self):
"""Checks whether the file is a TTY using an open-only optimization.

TTYs are always character devices. If the interpreter knows a file is
not a character device when it would call ``isatty``, can skip that
call. Inside ``open()`` there is a fresh stat result that contains that
information. Use the stat result to skip a system call. Outside of that
context TOCTOU issues (the fd could be arbitrarily modified by
surrounding code).
"""
if (self._stat_atopen is not None
and not stat.S_ISCHR(self._stat_atopen.st_mode)):
return True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be:

Suggested change
return True
return False

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, Py_RETURN_FALSE; I got right in the C but not the pyio. Making a new PR to update....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return os.isatty(self._fd)

@property
def closefd(self):
"""True if the file descriptor will be closed by close()."""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Skip the ``isatty`` system call during open() when the file is known to not
be a character device. This provides a slight performance improvement when
reading whole files.
2 changes: 1 addition & 1 deletion Modules/_io/_iomodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode,

/* buffering */
if (buffering < 0) {
PyObject *res = PyObject_CallMethodNoArgs(raw, &_Py_ID(isatty));
PyObject *res = PyObject_CallMethodNoArgs(raw, &_Py_ID(_isatty_open_only));
if (res == NULL)
goto error;
isatty = PyObject_IsTrue(res);
Expand Down
22 changes: 19 additions & 3 deletions Modules/_io/fileio.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@
#ifdef HAVE_SYS_TYPES_H
# include <sys/types.h>
#endif
#ifdef HAVE_SYS_STAT_H
vstinner marked this conversation as resolved.
Show resolved Hide resolved
# include <sys/stat.h>
#endif
#ifdef HAVE_IO_H
# include <io.h>
#endif
Expand Down Expand Up @@ -1218,6 +1215,24 @@ _io_FileIO_isatty_impl(fileio *self)
return PyBool_FromLong(res);
}

/* Checks whether the file is a TTY using an open-only optimization.

TTYs are always character devices. If the interpreter knows a file is
not a character device when it would call ``isatty``, can skip that
call. Inside ``open()`` there is a fresh stat result that contains that
information. Use the stat result to skip a system call. Outside of that
context TOCTOU issues (the fd could be arbitrarily modified by
surrounding code). */
static PyObject *
_io_FileIO_isatty_open_only(PyObject *op, PyObject *Py_UNUSED(ignored))
{
fileio *self = _PyFileIO_CAST(op);
if (self->stat_atopen != NULL && !S_ISCHR(self->stat_atopen->st_mode)) {
Py_RETURN_FALSE;
}
return _io_FileIO_isatty_impl(self);
}

#include "clinic/fileio.c.h"

static PyMethodDef fileio_methods[] = {
Expand All @@ -1234,6 +1249,7 @@ static PyMethodDef fileio_methods[] = {
_IO_FILEIO_WRITABLE_METHODDEF
_IO_FILEIO_FILENO_METHODDEF
_IO_FILEIO_ISATTY_METHODDEF
{"_isatty_open_only", _io_FileIO_isatty_open_only, METH_NOARGS},
{"_dealloc_warn", fileio_dealloc_warn, METH_O, NULL},
{"__reduce__", _PyIOBase_cannot_pickle, METH_NOARGS},
{"__reduce_ex__", _PyIOBase_cannot_pickle, METH_O},
Expand Down
1 change: 1 addition & 0 deletions Modules/_io/winconsoleio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,7 @@ static PyMethodDef winconsoleio_methods[] = {
_IO__WINDOWSCONSOLEIO_WRITABLE_METHODDEF
_IO__WINDOWSCONSOLEIO_FILENO_METHODDEF
_IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF
{"_isatty_open_only", (PyCFunction)_io__WindowsConsoleIO_isatty, METH_NOARGS},
{NULL, NULL} /* sentinel */
};

Expand Down
Loading