Skip to content

Commit 830daae

Browse files
nirsvstinner
authored andcommitted
[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.
1 parent 12fa6b1 commit 830daae

File tree

3 files changed

+66
-14
lines changed

3 files changed

+66
-14
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Creating io.FileIO() and builtin file() objects now release the GIL when
2+
checking the file descriptor. io.FileIO.readall(), io.FileIO.read(), and
3+
file.read() now release the GIL when getting the file size. Fixed hang of all
4+
threads with inaccessible NFS server. Patch by Nir Soffer.

Modules/_io/fileio.c

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,15 @@ dircheck(fileio* self, PyObject *nameobj)
146146
{
147147
#if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR)
148148
struct stat buf;
149+
int res;
149150
if (self->fd < 0)
150151
return 0;
151-
if (fstat(self->fd, &buf) == 0 && S_ISDIR(buf.st_mode)) {
152+
153+
Py_BEGIN_ALLOW_THREADS
154+
res = fstat(self->fd, &buf);
155+
Py_END_ALLOW_THREADS
156+
157+
if (res == 0 && S_ISDIR(buf.st_mode)) {
152158
errno = EISDIR;
153159
PyErr_SetFromErrnoWithFilenameObject(PyExc_IOError, nameobj);
154160
return -1;
@@ -162,17 +168,34 @@ check_fd(int fd)
162168
{
163169
#if defined(HAVE_FSTAT)
164170
struct stat buf;
165-
if (!_PyVerify_fd(fd) || (fstat(fd, &buf) < 0 && errno == EBADF)) {
166-
PyObject *exc;
167-
char *msg = strerror(EBADF);
168-
exc = PyObject_CallFunction(PyExc_OSError, "(is)",
169-
EBADF, msg);
170-
PyErr_SetObject(PyExc_OSError, exc);
171-
Py_XDECREF(exc);
172-
return -1;
171+
int res;
172+
PyObject *exc;
173+
char *msg;
174+
175+
if (!_PyVerify_fd(fd)) {
176+
goto badfd;
173177
}
174-
#endif
178+
179+
Py_BEGIN_ALLOW_THREADS
180+
res = fstat(fd, &buf);
181+
Py_END_ALLOW_THREADS
182+
183+
if (res < 0 && errno == EBADF) {
184+
goto badfd;
185+
}
186+
175187
return 0;
188+
189+
badfd:
190+
msg = strerror(EBADF);
191+
exc = PyObject_CallFunction(PyExc_OSError, "(is)",
192+
EBADF, msg);
193+
PyErr_SetObject(PyExc_OSError, exc);
194+
Py_XDECREF(exc);
195+
return -1;
196+
#else
197+
return 0;
198+
#endif
176199
}
177200

178201

@@ -519,9 +542,19 @@ new_buffersize(fileio *self, size_t currentsize)
519542
#ifdef HAVE_FSTAT
520543
off_t pos, end;
521544
struct stat st;
522-
if (fstat(self->fd, &st) == 0) {
545+
int res;
546+
547+
Py_BEGIN_ALLOW_THREADS
548+
res = fstat(self->fd, &st);
549+
Py_END_ALLOW_THREADS
550+
551+
if (res == 0) {
523552
end = st.st_size;
553+
554+
Py_BEGIN_ALLOW_THREADS
524555
pos = lseek(self->fd, 0L, SEEK_CUR);
556+
Py_END_ALLOW_THREADS
557+
525558
/* Files claiming a size smaller than SMALLCHUNK may
526559
actually be streaming pseudo-files. In this case, we
527560
apply the more aggressive algorithm below.

Objects/fileobject.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,15 @@ dircheck(PyFileObject* f)
121121
{
122122
#if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR)
123123
struct stat buf;
124+
int res;
124125
if (f->f_fp == NULL)
125126
return f;
126-
if (fstat(fileno(f->f_fp), &buf) == 0 &&
127-
S_ISDIR(buf.st_mode)) {
127+
128+
Py_BEGIN_ALLOW_THREADS
129+
res = fstat(fileno(f->f_fp), &buf);
130+
Py_END_ALLOW_THREADS
131+
132+
if (res == 0 && S_ISDIR(buf.st_mode)) {
128133
char *msg = strerror(EISDIR);
129134
PyObject *exc = PyObject_CallFunction(PyExc_IOError, "(isO)",
130135
EISDIR, msg, f->f_name);
@@ -1010,7 +1015,13 @@ new_buffersize(PyFileObject *f, size_t currentsize)
10101015
#ifdef HAVE_FSTAT
10111016
off_t pos, end;
10121017
struct stat st;
1013-
if (fstat(fileno(f->f_fp), &st) == 0) {
1018+
int res;
1019+
1020+
Py_BEGIN_ALLOW_THREADS
1021+
res = fstat(fileno(f->f_fp), &st);
1022+
Py_END_ALLOW_THREADS
1023+
1024+
if (res == 0) {
10141025
end = st.st_size;
10151026
/* The following is not a bug: we really need to call lseek()
10161027
*and* ftell(). The reason is that some stdio libraries
@@ -1021,7 +1032,11 @@ new_buffersize(PyFileObject *f, size_t currentsize)
10211032
works. We can't use the lseek() value either, because we
10221033
need to take the amount of buffered data into account.
10231034
(Yet another reason why stdio stinks. :-) */
1035+
1036+
Py_BEGIN_ALLOW_THREADS
10241037
pos = lseek(fileno(f->f_fp), 0L, SEEK_CUR);
1038+
Py_END_ALLOW_THREADS
1039+
10251040
if (pos >= 0) {
10261041
pos = ftell(f->f_fp);
10271042
}

0 commit comments

Comments
 (0)