[Python-checkins] [2.7] bpo-32186: Release the GIL during fstat and lseek calls (#4651)

Thu Dec 7 15:25:42 EST 2017

https://github.com/python/cpython/commit/830daae1c82ed33deef0086b7b6323e5be0b0cc8
commit: 830daae1c82ed33deef0086b7b6323e5be0b0cc8
branch: 2.7
author: Nir Soffer <nirsof at gmail.com>
committer: Victor Stinner <victor.stinner at gmail.com>
date: 2017年12月07日T21:25:39+01:00
summary:
[2.7] bpo-32186: Release the GIL during fstat and lseek calls (#4651)
In fileio, there were 3 fstat() calls and one lseek() call that did not
release the GIL during the call. This can cause all threads to hang for
unlimited time when using io.FileIO with inaccessible NFS server.
Same issue seen in fileio exists also in fileobject, fixed in the same
way.
files:
A Misc/NEWS.d/next/Library/2017-11-30-20-33-22.bpo-32186.O42bVe.rst
M Modules/_io/fileio.c
M Objects/fileobject.c

diff --git a/Misc/NEWS.d/next/Library/2017-11-30-20-33-22.bpo-32186.O42bVe.rst b/Misc/NEWS.d/next/Library/2017-11-30-20-33-22.bpo-32186.O42bVe.rst
new file mode 100644
index 00000000000..66c4468e80c
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-11-30-20-33-22.bpo-32186.O42bVe.rst
@@ -0,0 +1,4 @@
+Creating io.FileIO() and builtin file() objects now release the GIL when
+checking the file descriptor. io.FileIO.readall(), io.FileIO.read(), and
+file.read() now release the GIL when getting the file size. Fixed hang of all
+threads with inaccessible NFS server. Patch by Nir Soffer.
diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
index 4a71a57ec0d..2b40ada195a 100644
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -146,9 +146,15 @@ dircheck(fileio* self, PyObject *nameobj)
 {
 #if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR)
 struct stat buf;
+ int res;
 if (self->fd < 0)
 return 0;
- if (fstat(self->fd, &buf) == 0 && S_ISDIR(buf.st_mode)) {
+
+ Py_BEGIN_ALLOW_THREADS
+ res = fstat(self->fd, &buf);
+ Py_END_ALLOW_THREADS
+
+ if (res == 0 && S_ISDIR(buf.st_mode)) {
 errno = EISDIR;
 PyErr_SetFromErrnoWithFilenameObject(PyExc_IOError, nameobj);
 return -1;
@@ -162,17 +168,34 @@ check_fd(int fd)
 {
 #if defined(HAVE_FSTAT)
 struct stat buf;
- if (!_PyVerify_fd(fd) || (fstat(fd, &buf) < 0 && errno == EBADF)) {
- PyObject *exc;
- char *msg = strerror(EBADF);
- exc = PyObject_CallFunction(PyExc_OSError, "(is)",
- EBADF, msg);
- PyErr_SetObject(PyExc_OSError, exc);
- Py_XDECREF(exc);
- return -1;
+ int res;
+ PyObject *exc;
+ char *msg;
+
+ if (!_PyVerify_fd(fd)) {
+ goto badfd;
 }
-#endif
+
+ Py_BEGIN_ALLOW_THREADS
+ res = fstat(fd, &buf);
+ Py_END_ALLOW_THREADS
+
+ if (res < 0 && errno == EBADF) {
+ goto badfd;
+ }
+
 return 0;
+
+badfd:
+ msg = strerror(EBADF);
+ exc = PyObject_CallFunction(PyExc_OSError, "(is)",
+ EBADF, msg);
+ PyErr_SetObject(PyExc_OSError, exc);
+ Py_XDECREF(exc);
+ return -1;
+#else
+ return 0;
+#endif
 }
 
 
@@ -519,9 +542,19 @@ new_buffersize(fileio *self, size_t currentsize)
 #ifdef HAVE_FSTAT
 off_t pos, end;
 struct stat st;
- if (fstat(self->fd, &st) == 0) {
+ int res;
+
+ Py_BEGIN_ALLOW_THREADS
+ res = fstat(self->fd, &st);
+ Py_END_ALLOW_THREADS
+
+ if (res == 0) {
 end = st.st_size;
+
+ Py_BEGIN_ALLOW_THREADS
 pos = lseek(self->fd, 0L, SEEK_CUR);
+ Py_END_ALLOW_THREADS
+
 /* Files claiming a size smaller than SMALLCHUNK may
 actually be streaming pseudo-files. In this case, we
 apply the more aggressive algorithm below.
diff --git a/Objects/fileobject.c b/Objects/fileobject.c
index 2f63c374d1e..8d1c5812f0d 100644
--- a/Objects/fileobject.c
+++ b/Objects/fileobject.c
@@ -121,10 +121,15 @@ dircheck(PyFileObject* f)
 {
 #if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR)
 struct stat buf;
+ int res;
 if (f->f_fp == NULL)
 return f;
- if (fstat(fileno(f->f_fp), &buf) == 0 &&
- S_ISDIR(buf.st_mode)) {
+
+ Py_BEGIN_ALLOW_THREADS
+ res = fstat(fileno(f->f_fp), &buf);
+ Py_END_ALLOW_THREADS
+
+ if (res == 0 && S_ISDIR(buf.st_mode)) {
 char *msg = strerror(EISDIR);
 PyObject *exc = PyObject_CallFunction(PyExc_IOError, "(isO)",
 EISDIR, msg, f->f_name);
@@ -1010,7 +1015,13 @@ new_buffersize(PyFileObject *f, size_t currentsize)
 #ifdef HAVE_FSTAT
 off_t pos, end;
 struct stat st;
- if (fstat(fileno(f->f_fp), &st) == 0) {
+ int res;
+
+ Py_BEGIN_ALLOW_THREADS
+ res = fstat(fileno(f->f_fp), &st);
+ Py_END_ALLOW_THREADS
+
+ if (res == 0) {
 end = st.st_size;
 /* The following is not a bug: we really need to call lseek()
 *and* ftell(). The reason is that some stdio libraries
@@ -1021,7 +1032,11 @@ new_buffersize(PyFileObject *f, size_t currentsize)
 works. We can't use the lseek() value either, because we
 need to take the amount of buffered data into account.
 (Yet another reason why stdio stinks. :-) */
+
+ Py_BEGIN_ALLOW_THREADS
 pos = lseek(fileno(f->f_fp), 0L, SEEK_CUR);
+ Py_END_ALLOW_THREADS
+
 if (pos >= 0) {
 pos = ftell(f->f_fp);
 }
