From 56cce1ca84344f519f7ed01024434c083031d354 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Tue, 20 Mar 2018 12:16:30 -0700 Subject: [PATCH] [3.7] bpo-33021: Release the GIL during fstat() calls (GH-6019) (GH-6159) fstat may block for long time if the file descriptor is on a non-responsive NFS server, hanging all threads. Most fstat() calls are handled by _Py_fstat(), releasing the GIL internally, but but _Py_fstat_noraise() does not release the GIL, and most calls release the GIL explicitly around it. This patch fixes last 2 calls to _Py_fstat_no_raise(), avoiding hangs when calling: - mmap.mmap() - os.urandom() - random.seed() (cherry picked from commit 4484f9dca9149da135bbae035f10a50d20d1cbbb) Co-authored-by: Nir Soffer --- .../Library/2018-03-12-00-27-56.bpo-33021.m19B9T.rst | 2 ++ Modules/mmapmodule.c | 11 +++++++++-- Python/bootstrap_hash.c | 7 ++++++- 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-03-12-00-27-56.bpo-33021.m19B9T.rst diff --git a/Misc/NEWS.d/next/Library/2018-03-12-00-27-56.bpo-33021.m19B9T.rst b/Misc/NEWS.d/next/Library/2018-03-12-00-27-56.bpo-33021.m19B9T.rst new file mode 100644 index 00000000000..50dfafe600d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-03-12-00-27-56.bpo-33021.m19B9T.rst @@ -0,0 +1,2 @@ +Release the GIL during fstat() calls, avoiding hang of all threads when +calling mmap.mmap(), os.urandom(), and random.seed(). Patch by Nir Soffer. diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 6cf454573e9..95d42d6dc5e 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -1050,6 +1050,7 @@ static PyObject * new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) { struct _Py_stat_struct status; + int fstat_result = -1; mmap_object *m_obj; Py_ssize_t map_size; off_t offset = 0; @@ -1115,8 +1116,14 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) if (fd != -1) (void)fcntl(fd, F_FULLFSYNC); #endif - if (fd != -1 && _Py_fstat_noraise(fd, &status) == 0 - && S_ISREG(status.st_mode)) { + + if (fd != -1) { + Py_BEGIN_ALLOW_THREADS + fstat_result = _Py_fstat_noraise(fd, &status); + Py_END_ALLOW_THREADS + } + + if (fd != -1 && fstat_result == 0 && S_ISREG(status.st_mode)) { if (map_size == 0) { if (status.st_size == 0) { PyErr_SetString(PyExc_ValueError, diff --git a/Python/bootstrap_hash.c b/Python/bootstrap_hash.c index 9fd5cfb200e..073b2ea8dbd 100644 --- a/Python/bootstrap_hash.c +++ b/Python/bootstrap_hash.c @@ -301,10 +301,15 @@ dev_urandom(char *buffer, Py_ssize_t size, int raise) if (raise) { struct _Py_stat_struct st; + int fstat_result; if (urandom_cache.fd >= 0) { + Py_BEGIN_ALLOW_THREADS + fstat_result = _Py_fstat_noraise(urandom_cache.fd, &st); + Py_END_ALLOW_THREADS + /* Does the fd point to the same thing as before? (issue #21207) */ - if (_Py_fstat_noraise(urandom_cache.fd, &st) + if (fstat_result || st.st_dev != urandom_cache.st_dev || st.st_ino != urandom_cache.st_ino) { /* Something changed: forget the cached fd (but don't close it,