Issue #23694: Enhance _Py_open(), it now raises exceptions
* _Py_open() now raises exceptions on error. If open() fails, it raises an
OSError with the filename.
* _Py_open() now releases the GIL while calling open()
* Add _Py_open_noraise() when _Py_open() cannot be used because the GIL is not
held
diff --git a/Include/fileutils.h b/Include/fileutils.h
index 95632ed..f5fd668 100644
--- a/Include/fileutils.h
+++ b/Include/fileutils.h
@@ -62,6 +62,10 @@
PyAPI_FUNC(int) _Py_open(
const char *pathname,
int flags);
+
+PyAPI_FUNC(int) _Py_open_noraise(
+ const char *pathname,
+ int flags);
#endif
PyAPI_FUNC(FILE *) _Py_wfopen(
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index a8c2be2..a33df21 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -257,6 +257,7 @@
fd_dir_fd = _Py_open(FD_DIR, O_RDONLY);
if (fd_dir_fd == -1) {
/* No way to get a list of open fds. */
+ PyErr_Clear();
_close_fds_by_brute_force(start_fd, py_fds_to_keep);
return;
} else {
diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c
index ac134b8..7c4d17f 100644
--- a/Modules/mmapmodule.c
+++ b/Modules/mmapmodule.c
@@ -1221,7 +1221,6 @@
fd = devzero = _Py_open("/dev/zero", O_RDWR);
if (devzero == -1) {
Py_DECREF(m_obj);
- PyErr_SetFromErrno(PyExc_OSError);
return NULL;
}
#endif
diff --git a/Modules/ossaudiodev.c b/Modules/ossaudiodev.c
index 2d0dd09..34e68c7 100644
--- a/Modules/ossaudiodev.c
+++ b/Modules/ossaudiodev.c
@@ -116,11 +116,8 @@
provides a special ioctl() for non-blocking read/write, which is
exposed via oss_nonblock() below. */
fd = _Py_open(devicename, imode|O_NONBLOCK);
-
- if (fd == -1) {
- PyErr_SetFromErrnoWithFilename(PyExc_IOError, devicename);
+ if (fd == -1)
return NULL;
- }
/* And (try to) put it back in blocking mode so we get the
expected write() semantics. */
@@ -180,10 +177,8 @@
}
fd = _Py_open(devicename, O_RDWR);
- if (fd == -1) {
- PyErr_SetFromErrnoWithFilename(PyExc_IOError, devicename);
+ if (fd == -1)
return NULL;
- }
if ((self = PyObject_New(oss_mixer_t, &OSSMixerType)) == NULL) {
close(fd);
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index e47bd84..b8151c3 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -7930,7 +7930,7 @@
slave_fd = _Py_open(slave_name, O_RDWR);
if (slave_fd < 0)
- goto posix_error;
+ goto error;
#else
master_fd = open(DEV_PTY_FILE, O_RDWR | O_NOCTTY); /* open master */
@@ -7958,8 +7958,8 @@
goto posix_error;
slave_fd = _Py_open(slave_name, O_RDWR | O_NOCTTY); /* open slave */
- if (slave_fd < 0)
- goto posix_error;
+ if (slave_fd == -1)
+ goto error;
if (_Py_set_inheritable(master_fd, 0, NULL) < 0)
goto posix_error;
@@ -7977,9 +7977,7 @@
posix_error:
posix_error();
-#if defined(HAVE_OPENPTY) || defined(HAVE__GETPTY)
error:
-#endif
if (master_fd != -1)
close(master_fd);
if (slave_fd != -1)
diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c
index ffaf865..ef53067 100644
--- a/Modules/selectmodule.c
+++ b/Modules/selectmodule.c
@@ -1013,7 +1013,6 @@
struct pollfd *fds;
struct rlimit limit;
- Py_BEGIN_ALLOW_THREADS
/*
** If we try to process more that getrlimit()
** fds, the kernel will give an error, so
@@ -1021,18 +1020,14 @@
** value, because we can change rlimit() anytime.
*/
limit_result = getrlimit(RLIMIT_NOFILE, &limit);
- if (limit_result != -1)
- fd_devpoll = _Py_open("/dev/poll", O_RDWR);
- Py_END_ALLOW_THREADS
-
if (limit_result == -1) {
PyErr_SetFromErrno(PyExc_OSError);
return NULL;
}
- if (fd_devpoll == -1) {
- PyErr_SetFromErrnoWithFilename(PyExc_IOError, "/dev/poll");
+
+ fd_devpoll = _Py_open("/dev/poll", O_RDWR);
+ if (fd_devpoll == -1)
return NULL;
- }
fds = PyMem_NEW(struct pollfd, limit.rlim_cur);
if (fds == NULL) {
diff --git a/Python/fileutils.c b/Python/fileutils.c
index c6cdb19..7686040 100644
--- a/Python/fileutils.c
+++ b/Python/fileutils.c
@@ -30,7 +30,8 @@
0: open() ignores O_CLOEXEC flag, ex: Linux kernel older than 2.6.23
1: open() supports O_CLOEXEC flag, close-on-exec is set
- The flag is used by _Py_open(), io.FileIO and os.open() */
+ The flag is used by _Py_open(), _Py_open_noraise(), io.FileIO
+ and os.open(). */
int _Py_open_cloexec_works = -1;
#endif
@@ -907,37 +908,74 @@
return set_inheritable(fd, inheritable, 1, atomic_flag_works);
}
-/* Open a file with the specified flags (wrapper to open() function).
- The file descriptor is created non-inheritable. */
-int
-_Py_open(const char *pathname, int flags)
+static int
+_Py_open_impl(const char *pathname, int flags, int gil_held)
{
int fd;
-#ifdef MS_WINDOWS
- fd = open(pathname, flags | O_NOINHERIT);
- if (fd < 0)
- return fd;
-#else
-
+#ifndef MS_WINDOWS
int *atomic_flag_works;
-#ifdef O_CLOEXEC
+#endif
+
+#ifdef MS_WINDOWS
+ flags |= O_NOINHERIT;
+#elif defined(O_CLOEXEC)
atomic_flag_works = &_Py_open_cloexec_works;
flags |= O_CLOEXEC;
#else
atomic_flag_works = NULL;
#endif
- fd = open(pathname, flags);
- if (fd < 0)
- return fd;
- if (set_inheritable(fd, 0, 0, atomic_flag_works) < 0) {
+ if (gil_held) {
+ Py_BEGIN_ALLOW_THREADS
+ fd = open(pathname, flags);
+ Py_END_ALLOW_THREADS
+
+ if (fd < 0) {
+ PyErr_SetFromErrnoWithFilename(PyExc_OSError, pathname);
+ return -1;
+ }
+ }
+ else {
+ fd = open(pathname, flags);
+ if (fd < 0)
+ return -1;
+ }
+
+#ifndef MS_WINDOWS
+ if (set_inheritable(fd, 0, gil_held, atomic_flag_works) < 0) {
close(fd);
return -1;
}
-#endif /* !MS_WINDOWS */
+#endif
+
return fd;
}
+/* Open a file with the specified flags (wrapper to open() function).
+ Return a file descriptor on success. Raise an exception and return -1 on
+ error.
+
+ The file descriptor is created non-inheritable.
+
+ The GIL must be held. Use _Py_open_noraise() if the GIL cannot be held. */
+int
+_Py_open(const char *pathname, int flags)
+{
+ /* _Py_open() must be called with the GIL held. */
+ assert(PyGILState_Check());
+ return _Py_open_impl(pathname, flags, 1);
+}
+
+/* Open a file with the specified flags (wrapper to open() function).
+ Return a file descriptor on success. Set errno and return -1 on error.
+
+ The file descriptor is created non-inheritable. */
+int
+_Py_open_noraise(const char *pathname, int flags)
+{
+ return _Py_open_impl(pathname, flags, 0);
+}
+
/* Open a file. Use _wfopen() on Windows, encode the path to the locale
encoding and use fopen() otherwise. The file descriptor is created
non-inheritable. */
diff --git a/Python/random.c b/Python/random.c
index 031d887..39a389c 100644
--- a/Python/random.c
+++ b/Python/random.c
@@ -111,7 +111,7 @@
assert (0 < size);
- fd = _Py_open("/dev/urandom", O_RDONLY);
+ fd = _Py_open_noraise("/dev/urandom", O_RDONLY);
if (fd < 0)
Py_FatalError("Failed to open /dev/urandom");
@@ -158,17 +158,13 @@
if (urandom_cache.fd >= 0)
fd = urandom_cache.fd;
else {
- Py_BEGIN_ALLOW_THREADS
fd = _Py_open("/dev/urandom", O_RDONLY);
- Py_END_ALLOW_THREADS
- if (fd < 0)
- {
+ if (fd < 0) {
if (errno == ENOENT || errno == ENXIO ||
errno == ENODEV || errno == EACCES)
PyErr_SetString(PyExc_NotImplementedError,
"/dev/urandom (or equivalent) not found");
- else
- PyErr_SetFromErrno(PyExc_OSError);
+ /* otherwise, keep the OSError exception raised by _Py_open() */
return -1;
}
if (urandom_cache.fd >= 0) {