mirror of
https://github.com/python/cpython.git
synced 2025-08-22 17:55:18 +00:00
gh-118422: Fix run_fileexflags() test (#118429)
Don't test the undefined behavior of fileno() on a closed file, but use fstat() as a reliable test if the file was closed or not.
This commit is contained in:
parent
587388ff22
commit
e93c39b47e
4 changed files with 62 additions and 58 deletions
|
@ -326,6 +326,9 @@ extern int _PyFile_Flush(PyObject *);
|
||||||
extern int _Py_GetTicksPerSecond(long *ticks_per_second);
|
extern int _Py_GetTicksPerSecond(long *ticks_per_second);
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
// Export for '_testcapi' shared extension
|
||||||
|
PyAPI_FUNC(int) _Py_IsValidFD(int fd);
|
||||||
|
|
||||||
#ifdef __cplusplus
|
#ifdef __cplusplus
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
|
@ -1,5 +1,7 @@
|
||||||
|
#define PYTESTCAPI_NEED_INTERNAL_API
|
||||||
#include "parts.h"
|
#include "parts.h"
|
||||||
#include "util.h"
|
#include "util.h"
|
||||||
|
#include "pycore_fileutils.h" // _Py_IsValidFD()
|
||||||
|
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
#include <errno.h>
|
#include <errno.h>
|
||||||
|
@ -71,21 +73,18 @@ run_fileexflags(PyObject *mod, PyObject *pos_args)
|
||||||
PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename);
|
PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
int fd = fileno(fp);
|
||||||
|
|
||||||
result = PyRun_FileExFlags(fp, filename, start, globals, locals, closeit, pflags);
|
result = PyRun_FileExFlags(fp, filename, start, globals, locals, closeit, pflags);
|
||||||
|
|
||||||
#if defined(__linux__) || defined(MS_WINDOWS) || defined(__APPLE__)
|
if (closeit && result && _Py_IsValidFD(fd)) {
|
||||||
/* The behavior of fileno() after fclose() is undefined, but it is
|
|
||||||
* the only practical way to check whether the file was closed.
|
|
||||||
* Only test this on the known platforms. */
|
|
||||||
if (closeit && result && fileno(fp) >= 0) {
|
|
||||||
PyErr_SetString(PyExc_AssertionError, "File was not closed after excution");
|
PyErr_SetString(PyExc_AssertionError, "File was not closed after excution");
|
||||||
Py_DECREF(result);
|
Py_DECREF(result);
|
||||||
fclose(fp);
|
fclose(fp);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
#endif
|
|
||||||
if (!closeit && fileno(fp) < 0) {
|
if (!closeit && !_Py_IsValidFD(fd)) {
|
||||||
PyErr_SetString(PyExc_AssertionError, "Bad file descriptor after excution");
|
PyErr_SetString(PyExc_AssertionError, "Bad file descriptor after excution");
|
||||||
Py_XDECREF(result);
|
Py_XDECREF(result);
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|
|
@ -3050,3 +3050,52 @@ _Py_GetTicksPerSecond(long *ticks_per_second)
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
|
||||||
|
/* Check if a file descriptor is valid or not.
|
||||||
|
Return 0 if the file descriptor is invalid, return non-zero otherwise. */
|
||||||
|
int
|
||||||
|
_Py_IsValidFD(int fd)
|
||||||
|
{
|
||||||
|
/* dup() is faster than fstat(): fstat() can require input/output operations,
|
||||||
|
whereas dup() doesn't. There is a low risk of EMFILE/ENFILE at Python
|
||||||
|
startup. Problem: dup() doesn't check if the file descriptor is valid on
|
||||||
|
some platforms.
|
||||||
|
|
||||||
|
fcntl(fd, F_GETFD) is even faster, because it only checks the process table.
|
||||||
|
It is preferred over dup() when available, since it cannot fail with the
|
||||||
|
"too many open files" error (EMFILE).
|
||||||
|
|
||||||
|
bpo-30225: On macOS Tiger, when stdout is redirected to a pipe and the other
|
||||||
|
side of the pipe is closed, dup(1) succeed, whereas fstat(1, &st) fails with
|
||||||
|
EBADF. FreeBSD has similar issue (bpo-32849).
|
||||||
|
|
||||||
|
Only use dup() on Linux where dup() is enough to detect invalid FD
|
||||||
|
(bpo-32849).
|
||||||
|
*/
|
||||||
|
if (fd < 0) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
#if defined(F_GETFD) && ( \
|
||||||
|
defined(__linux__) || \
|
||||||
|
defined(__APPLE__) || \
|
||||||
|
(defined(__wasm__) && !defined(__wasi__)))
|
||||||
|
return fcntl(fd, F_GETFD) >= 0;
|
||||||
|
#elif defined(__linux__)
|
||||||
|
int fd2 = dup(fd);
|
||||||
|
if (fd2 >= 0) {
|
||||||
|
close(fd2);
|
||||||
|
}
|
||||||
|
return (fd2 >= 0);
|
||||||
|
#elif defined(MS_WINDOWS)
|
||||||
|
HANDLE hfile;
|
||||||
|
_Py_BEGIN_SUPPRESS_IPH
|
||||||
|
hfile = (HANDLE)_get_osfhandle(fd);
|
||||||
|
_Py_END_SUPPRESS_IPH
|
||||||
|
return (hfile != INVALID_HANDLE_VALUE
|
||||||
|
&& GetFileType(hfile) != FILE_TYPE_UNKNOWN);
|
||||||
|
#else
|
||||||
|
struct stat st;
|
||||||
|
return (fstat(fd, &st) == 0);
|
||||||
|
#endif
|
||||||
|
}
|
||||||
|
|
|
@ -2410,54 +2410,6 @@ init_import_site(void)
|
||||||
return _PyStatus_OK();
|
return _PyStatus_OK();
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Check if a file descriptor is valid or not.
|
|
||||||
Return 0 if the file descriptor is invalid, return non-zero otherwise. */
|
|
||||||
static int
|
|
||||||
is_valid_fd(int fd)
|
|
||||||
{
|
|
||||||
/* dup() is faster than fstat(): fstat() can require input/output operations,
|
|
||||||
whereas dup() doesn't. There is a low risk of EMFILE/ENFILE at Python
|
|
||||||
startup. Problem: dup() doesn't check if the file descriptor is valid on
|
|
||||||
some platforms.
|
|
||||||
|
|
||||||
fcntl(fd, F_GETFD) is even faster, because it only checks the process table.
|
|
||||||
It is preferred over dup() when available, since it cannot fail with the
|
|
||||||
"too many open files" error (EMFILE).
|
|
||||||
|
|
||||||
bpo-30225: On macOS Tiger, when stdout is redirected to a pipe and the other
|
|
||||||
side of the pipe is closed, dup(1) succeed, whereas fstat(1, &st) fails with
|
|
||||||
EBADF. FreeBSD has similar issue (bpo-32849).
|
|
||||||
|
|
||||||
Only use dup() on Linux where dup() is enough to detect invalid FD
|
|
||||||
(bpo-32849).
|
|
||||||
*/
|
|
||||||
if (fd < 0) {
|
|
||||||
return 0;
|
|
||||||
}
|
|
||||||
#if defined(F_GETFD) && ( \
|
|
||||||
defined(__linux__) || \
|
|
||||||
defined(__APPLE__) || \
|
|
||||||
defined(__wasm__))
|
|
||||||
return fcntl(fd, F_GETFD) >= 0;
|
|
||||||
#elif defined(__linux__)
|
|
||||||
int fd2 = dup(fd);
|
|
||||||
if (fd2 >= 0) {
|
|
||||||
close(fd2);
|
|
||||||
}
|
|
||||||
return (fd2 >= 0);
|
|
||||||
#elif defined(MS_WINDOWS)
|
|
||||||
HANDLE hfile;
|
|
||||||
_Py_BEGIN_SUPPRESS_IPH
|
|
||||||
hfile = (HANDLE)_get_osfhandle(fd);
|
|
||||||
_Py_END_SUPPRESS_IPH
|
|
||||||
return (hfile != INVALID_HANDLE_VALUE
|
|
||||||
&& GetFileType(hfile) != FILE_TYPE_UNKNOWN);
|
|
||||||
#else
|
|
||||||
struct stat st;
|
|
||||||
return (fstat(fd, &st) == 0);
|
|
||||||
#endif
|
|
||||||
}
|
|
||||||
|
|
||||||
/* returns Py_None if the fd is not valid */
|
/* returns Py_None if the fd is not valid */
|
||||||
static PyObject*
|
static PyObject*
|
||||||
create_stdio(const PyConfig *config, PyObject* io,
|
create_stdio(const PyConfig *config, PyObject* io,
|
||||||
|
@ -2471,8 +2423,9 @@ create_stdio(const PyConfig *config, PyObject* io,
|
||||||
int buffering, isatty;
|
int buffering, isatty;
|
||||||
const int buffered_stdio = config->buffered_stdio;
|
const int buffered_stdio = config->buffered_stdio;
|
||||||
|
|
||||||
if (!is_valid_fd(fd))
|
if (!_Py_IsValidFD(fd)) {
|
||||||
Py_RETURN_NONE;
|
Py_RETURN_NONE;
|
||||||
|
}
|
||||||
|
|
||||||
/* stdin is always opened in buffered mode, first because it shouldn't
|
/* stdin is always opened in buffered mode, first because it shouldn't
|
||||||
make a difference in common use cases, second because TextIOWrapper
|
make a difference in common use cases, second because TextIOWrapper
|
||||||
|
@ -2588,9 +2541,9 @@ error:
|
||||||
Py_XDECREF(text);
|
Py_XDECREF(text);
|
||||||
Py_XDECREF(raw);
|
Py_XDECREF(raw);
|
||||||
|
|
||||||
if (PyErr_ExceptionMatches(PyExc_OSError) && !is_valid_fd(fd)) {
|
if (PyErr_ExceptionMatches(PyExc_OSError) && !_Py_IsValidFD(fd)) {
|
||||||
/* Issue #24891: the file descriptor was closed after the first
|
/* Issue #24891: the file descriptor was closed after the first
|
||||||
is_valid_fd() check was called. Ignore the OSError and set the
|
_Py_IsValidFD() check was called. Ignore the OSError and set the
|
||||||
stream to None. */
|
stream to None. */
|
||||||
PyErr_Clear();
|
PyErr_Clear();
|
||||||
Py_RETURN_NONE;
|
Py_RETURN_NONE;
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue