bpo-30555: Fix WindowsConsoleIO fails in the presence of fd redirection (GH-1927)
This works by not caching the handle and instead getting the handle from
the file descriptor each time, so that if the actual handle changes by
fd redirection closing/opening the console handle beneath our feet, we
will keep working correctly.
diff --git a/Modules/_io/clinic/winconsoleio.c.h b/Modules/_io/clinic/winconsoleio.c.h
index cf6ce60..84e73dc 100644
--- a/Modules/_io/clinic/winconsoleio.c.h
+++ b/Modules/_io/clinic/winconsoleio.c.h
@@ -8,10 +8,10 @@ PyDoc_STRVAR(_io__WindowsConsoleIO_close__doc__,
"close($self, /)\n"
"--\n"
"\n"
-"Close the handle.\n"
+"Close the console object.\n"
"\n"
-"A closed handle cannot be used for further I/O operations. close() may be\n"
-"called more than once without error.");
+"A closed console object cannot be used for further I/O operations.\n"
+"close() may be called more than once without error.");
#define _IO__WINDOWSCONSOLEIO_CLOSE_METHODDEF \
{"close", (PyCFunction)_io__WindowsConsoleIO_close, METH_NOARGS, _io__WindowsConsoleIO_close__doc__},
@@ -110,10 +110,7 @@ PyDoc_STRVAR(_io__WindowsConsoleIO_fileno__doc__,
"fileno($self, /)\n"
"--\n"
"\n"
-"Return the underlying file descriptor (an integer).\n"
-"\n"
-"fileno is only set when a file descriptor is used to open\n"
-"one of the standard streams.");
+"Return the underlying file descriptor (an integer).");
#define _IO__WINDOWSCONSOLEIO_FILENO_METHODDEF \
{"fileno", (PyCFunction)_io__WindowsConsoleIO_fileno, METH_NOARGS, _io__WindowsConsoleIO_fileno__doc__},
@@ -381,4 +378,4 @@ _io__WindowsConsoleIO_isatty(winconsoleio *self, PyObject *Py_UNUSED(ignored))
#ifndef _IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF
#define _IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF
#endif /* !defined(_IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF) */
-/*[clinic end generated code: output=a28b3120fa53b256 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=48080572ffee22f5 input=a9049054013a1b77]*/
diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c
index 4ccf027..460f2d3 100644
--- a/Modules/_io/winconsoleio.c
+++ b/Modules/_io/winconsoleio.c
@@ -64,10 +64,7 @@ char _PyIO_get_console_type(PyObject *path_or_fd) {
int fd = PyLong_AsLong(path_or_fd);
PyErr_Clear();
if (fd >= 0) {
- HANDLE handle;
- _Py_BEGIN_SUPPRESS_IPH
- handle = (HANDLE)_get_osfhandle(fd);
- _Py_END_SUPPRESS_IPH
+ HANDLE handle = _Py_get_osfhandle_noraise(fd);
if (handle == INVALID_HANDLE_VALUE)
return '\0';
return _get_console_type(handle);
@@ -143,12 +140,11 @@ class _io._WindowsConsoleIO "winconsoleio *" "&PyWindowsConsoleIO_Type"
typedef struct {
PyObject_HEAD
- HANDLE handle;
int fd;
unsigned int created : 1;
unsigned int readable : 1;
unsigned int writable : 1;
- unsigned int closehandle : 1;
+ unsigned int closefd : 1;
char finalizing;
unsigned int blksize;
PyObject *weakreflist;
@@ -164,7 +160,7 @@ _Py_IDENTIFIER(name);
int
_PyWindowsConsoleIO_closed(PyObject *self)
{
- return ((winconsoleio *)self)->handle == INVALID_HANDLE_VALUE;
+ return ((winconsoleio *)self)->fd == -1;
}
@@ -172,16 +168,12 @@ _PyWindowsConsoleIO_closed(PyObject *self)
static int
internal_close(winconsoleio *self)
{
- if (self->handle != INVALID_HANDLE_VALUE) {
- if (self->closehandle) {
- if (self->fd >= 0) {
- _Py_BEGIN_SUPPRESS_IPH
- close(self->fd);
- _Py_END_SUPPRESS_IPH
- }
- CloseHandle(self->handle);
+ if (self->fd != -1) {
+ if (self->closefd) {
+ _Py_BEGIN_SUPPRESS_IPH
+ close(self->fd);
+ _Py_END_SUPPRESS_IPH
}
- self->handle = INVALID_HANDLE_VALUE;
self->fd = -1;
}
return 0;
@@ -190,15 +182,15 @@ internal_close(winconsoleio *self)
/*[clinic input]
_io._WindowsConsoleIO.close
-Close the handle.
+Close the console object.
-A closed handle cannot be used for further I/O operations. close() may be
-called more than once without error.
+A closed console object cannot be used for further I/O operations.
+close() may be called more than once without error.
[clinic start generated code]*/
static PyObject *
_io__WindowsConsoleIO_close_impl(winconsoleio *self)
-/*[clinic end generated code: output=27ef95b66c29057b input=185617e349ae4c7b]*/
+/*[clinic end generated code: output=27ef95b66c29057b input=68c4e5754f8136c2]*/
{
PyObject *res;
PyObject *exc, *val, *tb;
@@ -206,8 +198,8 @@ _io__WindowsConsoleIO_close_impl(winconsoleio *self)
_Py_IDENTIFIER(close);
res = _PyObject_CallMethodIdOneArg((PyObject*)&PyRawIOBase_Type,
&PyId_close, (PyObject*)self);
- if (!self->closehandle) {
- self->handle = INVALID_HANDLE_VALUE;
+ if (!self->closefd) {
+ self->fd = -1;
return res;
}
if (res == NULL)
@@ -229,12 +221,11 @@ winconsoleio_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
self = (winconsoleio *) type->tp_alloc(type, 0);
if (self != NULL) {
- self->handle = INVALID_HANDLE_VALUE;
self->fd = -1;
self->created = 0;
self->readable = 0;
self->writable = 0;
- self->closehandle = 0;
+ self->closefd = 0;
self->blksize = 0;
self->weakreflist = NULL;
}
@@ -269,16 +260,17 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
int rwa = 0;
int fd = -1;
int fd_is_own = 0;
+ HANDLE handle = NULL;
assert(PyWindowsConsoleIO_Check(self));
- if (self->handle >= 0) {
- if (self->closehandle) {
+ if (self->fd >= 0) {
+ if (self->closefd) {
/* Have to close the existing file first. */
if (internal_close(self) < 0)
return -1;
}
else
- self->handle = INVALID_HANDLE_VALUE;
+ self->fd = -1;
}
fd = _PyLong_AsInt(nameobj);
@@ -341,14 +333,12 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
goto bad_mode;
if (fd >= 0) {
- _Py_BEGIN_SUPPRESS_IPH
- self->handle = (HANDLE)_get_osfhandle(fd);
- _Py_END_SUPPRESS_IPH
- self->closehandle = 0;
+ handle = _Py_get_osfhandle_noraise(fd);
+ self->closefd = 0;
} else {
DWORD access = GENERIC_READ;
- self->closehandle = 1;
+ self->closefd = 1;
if (!closefd) {
PyErr_SetString(PyExc_ValueError,
"Cannot use closefd=False with file name");
@@ -363,21 +353,31 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
on the specific access. This is required for modern names
CONIN$ and CONOUT$, which allow reading/writing state as
well as reading/writing content. */
- self->handle = CreateFileW(name, GENERIC_READ | GENERIC_WRITE,
+ handle = CreateFileW(name, GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL);
- if (self->handle == INVALID_HANDLE_VALUE)
- self->handle = CreateFileW(name, access,
+ if (handle == INVALID_HANDLE_VALUE)
+ handle = CreateFileW(name, access,
FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL);
Py_END_ALLOW_THREADS
- if (self->handle == INVALID_HANDLE_VALUE) {
+ if (handle == INVALID_HANDLE_VALUE) {
PyErr_SetExcFromWindowsErrWithFilenameObject(PyExc_OSError, GetLastError(), nameobj);
goto error;
}
+
+ if (self->writable)
+ self->fd = _Py_open_osfhandle_noraise(handle, _O_WRONLY | _O_BINARY);
+ else
+ self->fd = _Py_open_osfhandle_noraise(handle, _O_RDONLY | _O_BINARY);
+ if (self->fd < 0) {
+ CloseHandle(handle);
+ PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
+ goto error;
+ }
}
if (console_type == '\0')
- console_type = _get_console_type(self->handle);
+ console_type = _get_console_type(handle);
if (self->writable && console_type != 'w') {
PyErr_SetString(PyExc_ValueError,
@@ -460,25 +460,14 @@ _io._WindowsConsoleIO.fileno
Return the underlying file descriptor (an integer).
-fileno is only set when a file descriptor is used to open
-one of the standard streams.
-
[clinic start generated code]*/
static PyObject *
_io__WindowsConsoleIO_fileno_impl(winconsoleio *self)
-/*[clinic end generated code: output=006fa74ce3b5cfbf input=079adc330ddaabe6]*/
+/*[clinic end generated code: output=006fa74ce3b5cfbf input=845c47ebbc3a2f67]*/
{
- if (self->fd < 0 && self->handle != INVALID_HANDLE_VALUE) {
- _Py_BEGIN_SUPPRESS_IPH
- if (self->writable)
- self->fd = _open_osfhandle((intptr_t)self->handle, _O_WRONLY | _O_BINARY);
- else
- self->fd = _open_osfhandle((intptr_t)self->handle, _O_RDONLY | _O_BINARY);
- _Py_END_SUPPRESS_IPH
- }
if (self->fd < 0)
- return err_mode("fileno");
+ return err_closed();
return PyLong_FromLong(self->fd);
}
@@ -492,7 +481,7 @@ static PyObject *
_io__WindowsConsoleIO_readable_impl(winconsoleio *self)
/*[clinic end generated code: output=daf9cef2743becf0 input=6be9defb5302daae]*/
{
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return err_closed();
return PyBool_FromLong((long) self->readable);
}
@@ -507,7 +496,7 @@ static PyObject *
_io__WindowsConsoleIO_writable_impl(winconsoleio *self)
/*[clinic end generated code: output=e0a2ad7eae5abf67 input=cefbd8abc24df6a0]*/
{
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return err_closed();
return PyBool_FromLong((long) self->writable);
}
@@ -642,7 +631,7 @@ read_console_w(HANDLE handle, DWORD maxlen, DWORD *readlen) {
static Py_ssize_t
readinto(winconsoleio *self, char *buf, Py_ssize_t len)
{
- if (self->handle == INVALID_HANDLE_VALUE) {
+ if (self->fd == -1) {
err_closed();
return -1;
}
@@ -657,6 +646,10 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len)
return -1;
}
+ HANDLE handle = _Py_get_osfhandle(self->fd);
+ if (handle == INVALID_HANDLE_VALUE)
+ return -1;
+
/* Each character may take up to 4 bytes in the final buffer.
This is highly conservative, but necessary to avoid
failure for any given Unicode input (e.g. \U0010ffff).
@@ -678,7 +671,7 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len)
return read_len;
DWORD n;
- wchar_t *wbuf = read_console_w(self->handle, wlen, &n);
+ wchar_t *wbuf = read_console_w(handle, wlen, &n);
if (wbuf == NULL)
return -1;
if (n == 0) {
@@ -784,10 +777,15 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self)
DWORD bufsize, n, len = 0;
PyObject *bytes;
DWORD bytes_size, rn;
+ HANDLE handle;
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return err_closed();
+ handle = _Py_get_osfhandle(self->fd);
+ if (handle == INVALID_HANDLE_VALUE)
+ return NULL;
+
bufsize = BUFSIZ;
buf = (wchar_t*)PyMem_Malloc((bufsize + 1) * sizeof(wchar_t));
@@ -819,7 +817,7 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self)
buf = tmp;
}
- subbuf = read_console_w(self->handle, bufsize - len, &n);
+ subbuf = read_console_w(handle, bufsize - len, &n);
if (subbuf == NULL) {
PyMem_Free(buf);
@@ -909,7 +907,7 @@ _io__WindowsConsoleIO_read_impl(winconsoleio *self, Py_ssize_t size)
PyObject *bytes;
Py_ssize_t bytes_size;
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return err_closed();
if (!self->readable)
return err_mode("reading");
@@ -959,12 +957,17 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b)
BOOL res = TRUE;
wchar_t *wbuf;
DWORD len, wlen, n = 0;
+ HANDLE handle;
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return err_closed();
if (!self->writable)
return err_mode("writing");
+ handle = _Py_get_osfhandle(self->fd);
+ if (handle == INVALID_HANDLE_VALUE)
+ return NULL;
+
if (!b->len) {
return PyLong_FromLong(0);
}
@@ -995,7 +998,7 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b)
Py_BEGIN_ALLOW_THREADS
wlen = MultiByteToWideChar(CP_UTF8, 0, b->buf, len, wbuf, wlen);
if (wlen) {
- res = WriteConsoleW(self->handle, wbuf, wlen, &n, NULL);
+ res = WriteConsoleW(handle, wbuf, wlen, &n, NULL);
if (res && n < wlen) {
/* Wrote fewer characters than expected, which means our
* len value may be wrong. So recalculate it from the
@@ -1027,15 +1030,15 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b)
static PyObject *
winconsoleio_repr(winconsoleio *self)
{
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return PyUnicode_FromFormat("<_io._WindowsConsoleIO [closed]>");
if (self->readable)
return PyUnicode_FromFormat("<_io._WindowsConsoleIO mode='rb' closefd=%s>",
- self->closehandle ? "True" : "False");
+ self->closefd ? "True" : "False");
if (self->writable)
return PyUnicode_FromFormat("<_io._WindowsConsoleIO mode='wb' closefd=%s>",
- self->closehandle ? "True" : "False");
+ self->closefd ? "True" : "False");
PyErr_SetString(PyExc_SystemError, "_WindowsConsoleIO has invalid mode");
return NULL;
@@ -1051,7 +1054,7 @@ static PyObject *
_io__WindowsConsoleIO_isatty_impl(winconsoleio *self)
/*[clinic end generated code: output=9eac09d287c11bd7 input=9b91591dbe356f86]*/
{
- if (self->handle == INVALID_HANDLE_VALUE)
+ if (self->fd == -1)
return err_closed();
Py_RETURN_TRUE;
@@ -1077,13 +1080,13 @@ static PyMethodDef winconsoleio_methods[] = {
static PyObject *
get_closed(winconsoleio *self, void *closure)
{
- return PyBool_FromLong((long)(self->handle == INVALID_HANDLE_VALUE));
+ return PyBool_FromLong((long)(self->fd == -1));
}
static PyObject *
get_closefd(winconsoleio *self, void *closure)
{
- return PyBool_FromLong((long)(self->closehandle));
+ return PyBool_FromLong((long)(self->closefd));
}
static PyObject *
diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c
index 1e66962..9a2542f 100644
--- a/Modules/mmapmodule.c
+++ b/Modules/mmapmodule.c
@@ -1370,13 +1370,10 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
*/
if (fileno != -1 && fileno != 0) {
/* Ensure that fileno is within the CRT's valid range */
- _Py_BEGIN_SUPPRESS_IPH
- fh = (HANDLE)_get_osfhandle(fileno);
- _Py_END_SUPPRESS_IPH
- if (fh==(HANDLE)-1) {
- PyErr_SetFromErrno(PyExc_OSError);
+ fh = _Py_get_osfhandle(fileno);
+ if (fh == INVALID_HANDLE_VALUE)
return NULL;
- }
+
/* Win9x appears to need us seeked to zero */
lseek(fileno, 0, SEEK_SET);
}
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index e754db7..ecd210e 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -10098,18 +10098,16 @@ os_pipe_impl(PyObject *module)
attr.bInheritHandle = FALSE;
Py_BEGIN_ALLOW_THREADS
- _Py_BEGIN_SUPPRESS_IPH
ok = CreatePipe(&read, &write, &attr, 0);
if (ok) {
- fds[0] = _open_osfhandle((intptr_t)read, _O_RDONLY);
- fds[1] = _open_osfhandle((intptr_t)write, _O_WRONLY);
+ fds[0] = _Py_open_osfhandle_noraise(read, _O_RDONLY);
+ fds[1] = _Py_open_osfhandle_noraise(write, _O_WRONLY);
if (fds[0] == -1 || fds[1] == -1) {
CloseHandle(read);
CloseHandle(write);
ok = 0;
}
}
- _Py_END_SUPPRESS_IPH
Py_END_ALLOW_THREADS
if (!ok)