path: root/dev-python/cherrypy/files/cherrypy-18.6.1-close-files.patch

From 94a2cc036203c6da55174ef3b105c0c875bbc79f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= <mgorny@gentoo.org>
Date: Mon, 31 Jan 2022 22:25:34 +0100
Subject: [PATCH] Use context managers to close files properly and fix tests on
 PyPy

Use context managers (`with`) to ensure that all open files are closed
correctly.  This resolves resource leaks and test failures with PyPy3.7.

The code prior to this change used four approaches for closing files:

1. Using a context manager (`with` clause).

2. Using a try/finally clause.

3. Closing the file in the same scope (unreliable: file object can leak
   on exception).

4. Not closing open files at all.

The last point is a real problem for PyPy since it does not GC
unreachable objects as aggressively as CPython does.  While leaving
a function scope on CPython causes the file objects private to it to
be destroyed (and therefore closed), in PyPy they can stay dangling
for some time.  When combines with buffered writes, this means that
writes can still remain pending after returning from function.

Using a context manager is a simple, consistent way to ensure that
the file object is closed once it is no longer needed.  In turn, this
guarantees that all pending writes will be performed upon function
return and the code won't be hiting race conditions between writing
a file and reading it afterwards.
---
 cherrypy/_cperror.py         |  3 ++-
 cherrypy/_cpmodpy.py         |  5 +----
 cherrypy/lib/auth_digest.py  | 13 ++++++------
 cherrypy/lib/covercp.py      | 40 ++++++++++++++++++------------------
 cherrypy/lib/reprconf.py     |  5 +----
 cherrypy/lib/sessions.py     | 10 ++-------
 cherrypy/process/plugins.py  |  3 ++-
 cherrypy/test/helper.py      |  3 ++-
 cherrypy/test/logtest.py     | 33 ++++++++++++++++-------------
 cherrypy/test/modfastcgi.py  |  5 +----
 cherrypy/test/modfcgid.py    |  5 +----
 cherrypy/test/modpy.py       |  5 +----
 cherrypy/test/modwsgi.py     |  5 +----
 cherrypy/test/test_core.py   |  5 ++---
 cherrypy/test/test_states.py | 11 +++++-----
 15 files changed, 67 insertions(+), 84 deletions(-)

diff --git a/cherrypy/_cperror.py b/cherrypy/_cperror.py
index 4e727682..ebf1dcf6 100644
--- a/cherrypy/_cperror.py
+++ b/cherrypy/_cperror.py
@@ -532,7 +532,8 @@ def get_error_page(status, **kwargs):
                     return result
             else:
                 # Load the template from this path.
-                template = io.open(error_page, newline='').read()
+                with io.open(error_page, newline='') as f:
+                    template = f.read()
         except Exception:
             e = _format_exception(*_exc_info())[-1]
             m = kwargs['message']
diff --git a/cherrypy/_cpmodpy.py b/cherrypy/_cpmodpy.py
index 0e608c48..a08f0ed9 100644
--- a/cherrypy/_cpmodpy.py
+++ b/cherrypy/_cpmodpy.py
@@ -339,11 +339,8 @@ LoadModule python_module modules/mod_python.so
                                      }
 
         mpconf = os.path.join(os.path.dirname(__file__), 'cpmodpy.conf')
-        f = open(mpconf, 'wb')
-        try:
+        with open(mpconf, 'wb') as f:
             f.write(conf_data)
-        finally:
-            f.close()
 
         response = read_process(self.apache_path, '-k start -f %s' % mpconf)
         self.ready = True
diff --git a/cherrypy/lib/auth_digest.py b/cherrypy/lib/auth_digest.py
index fbb5df64..981e9a5d 100644
--- a/cherrypy/lib/auth_digest.py
+++ b/cherrypy/lib/auth_digest.py
@@ -101,13 +101,12 @@ def get_ha1_file_htdigest(filename):
     """
     def get_ha1(realm, username):
         result = None
-        f = open(filename, 'r')
-        for line in f:
-            u, r, ha1 = line.rstrip().split(':')
-            if u == username and r == realm:
-                result = ha1
-                break
-        f.close()
+        with open(filename, 'r') as f:
+            for line in f:
+                u, r, ha1 = line.rstrip().split(':')
+                if u == username and r == realm:
+                    result = ha1
+                    break
         return result
 
     return get_ha1
diff --git a/cherrypy/lib/covercp.py b/cherrypy/lib/covercp.py
index 3e219713..005fafa5 100644
--- a/cherrypy/lib/covercp.py
+++ b/cherrypy/lib/covercp.py
@@ -334,26 +334,26 @@ class CoverStats(object):
         yield '</body></html>'
 
     def annotated_file(self, filename, statements, excluded, missing):
-        source = open(filename, 'r')
-        buffer = []
-        for lineno, line in enumerate(source.readlines()):
-            lineno += 1
-            line = line.strip('\n\r')
-            empty_the_buffer = True
-            if lineno in excluded:
-                template = TEMPLATE_LOC_EXCLUDED
-            elif lineno in missing:
-                template = TEMPLATE_LOC_NOT_COVERED
-            elif lineno in statements:
-                template = TEMPLATE_LOC_COVERED
-            else:
-                empty_the_buffer = False
-                buffer.append((lineno, line))
-            if empty_the_buffer:
-                for lno, pastline in buffer:
-                    yield template % (lno, cgi.escape(pastline))
-                buffer = []
-                yield template % (lineno, cgi.escape(line))
+        with open(filename, 'r') as source:
+            buffer = []
+            for lineno, line in enumerate(source.readlines()):
+                lineno += 1
+                line = line.strip('\n\r')
+                empty_the_buffer = True
+                if lineno in excluded:
+                    template = TEMPLATE_LOC_EXCLUDED
+                elif lineno in missing:
+                    template = TEMPLATE_LOC_NOT_COVERED
+                elif lineno in statements:
+                    template = TEMPLATE_LOC_COVERED
+                else:
+                    empty_the_buffer = False
+                    buffer.append((lineno, line))
+                if empty_the_buffer:
+                    for lno, pastline in buffer:
+                        yield template % (lno, cgi.escape(pastline))
+                    buffer = []
+                    yield template % (lineno, cgi.escape(line))
 
     @cherrypy.expose
     def report(self, name):
diff --git a/cherrypy/lib/reprconf.py b/cherrypy/lib/reprconf.py
index 3976652e..76381d7b 100644
--- a/cherrypy/lib/reprconf.py
+++ b/cherrypy/lib/reprconf.py
@@ -163,11 +163,8 @@ class Parser(configparser.ConfigParser):
             #     fp = open(filename)
             # except IOError:
             #     continue
-            fp = open(filename)
-            try:
+            with open(filename) as fp:
                 self._read(fp, filename)
-            finally:
-                fp.close()
 
     def as_dict(self, raw=False, vars=None):
         """Convert an INI file to a dictionary"""
diff --git a/cherrypy/lib/sessions.py b/cherrypy/lib/sessions.py
index 5b3328f2..0f56a4fa 100644
--- a/cherrypy/lib/sessions.py
+++ b/cherrypy/lib/sessions.py
@@ -516,11 +516,8 @@ class FileSession(Session):
         if path is None:
             path = self._get_file_path()
         try:
-            f = open(path, 'rb')
-            try:
+            with open(path, 'rb') as f:
                 return pickle.load(f)
-            finally:
-                f.close()
         except (IOError, EOFError):
             e = sys.exc_info()[1]
             if self.debug:
@@ -531,11 +528,8 @@ class FileSession(Session):
     def _save(self, expiration_time):
         assert self.locked, ('The session was saved without being locked.  '
                              "Check your tools' priority levels.")
-        f = open(self._get_file_path(), 'wb')
-        try:
+        with open(self._get_file_path(), 'wb') as f:
             pickle.dump((self._data, expiration_time), f, self.pickle_protocol)
-        finally:
-            f.close()
 
     def _delete(self):
         assert self.locked, ('The session deletion without being locked.  '
diff --git a/cherrypy/process/plugins.py b/cherrypy/process/plugins.py
index 2a9952de..e96fb1ce 100644
--- a/cherrypy/process/plugins.py
+++ b/cherrypy/process/plugins.py
@@ -436,7 +436,8 @@ class PIDFile(SimplePlugin):
         if self.finalized:
             self.bus.log('PID %r already written to %r.' % (pid, self.pidfile))
         else:
-            open(self.pidfile, 'wb').write(ntob('%s\n' % pid, 'utf8'))
+            with open(self.pidfile, 'wb') as f:
+                f.write(ntob('%s\n' % pid, 'utf8'))
             self.bus.log('PID %r written to %r.' % (pid, self.pidfile))
             self.finalized = True
     start.priority = 70
diff --git a/cherrypy/test/helper.py b/cherrypy/test/helper.py
index c1ca4535..cae49533 100644
--- a/cherrypy/test/helper.py
+++ b/cherrypy/test/helper.py
@@ -505,7 +505,8 @@ server.ssl_private_key: r'%s'
 
     def get_pid(self):
         if self.daemonize:
-            return int(open(self.pid_file, 'rb').read())
+            with open(self.pid_file, 'rb') as f:
+                return int(f.read())
         return self._proc.pid
 
     def join(self):
diff --git a/cherrypy/test/logtest.py b/cherrypy/test/logtest.py
index 344be987..112bdc25 100644
--- a/cherrypy/test/logtest.py
+++ b/cherrypy/test/logtest.py
@@ -97,7 +97,8 @@ class LogCase(object):
 
     def emptyLog(self):
         """Overwrite self.logfile with 0 bytes."""
-        open(self.logfile, 'wb').write('')
+        with open(self.logfile, 'wb') as f:
+            f.write('')
 
     def markLog(self, key=None):
         """Insert a marker line into the log and set self.lastmarker."""
@@ -105,10 +106,11 @@ class LogCase(object):
             key = str(time.time())
         self.lastmarker = key
 
-        open(self.logfile, 'ab+').write(
-            b'%s%s\n'
-            % (self.markerPrefix, key.encode('utf-8'))
-        )
+        with open(self.logfile, 'ab+') as f:
+            f.write(
+                b'%s%s\n'
+                % (self.markerPrefix, key.encode('utf-8'))
+            )
 
     def _read_marked_region(self, marker=None):
         """Return lines from self.logfile in the marked region.
@@ -122,20 +124,23 @@ class LogCase(object):
         logfile = self.logfile
         marker = marker or self.lastmarker
         if marker is None:
-            return open(logfile, 'rb').readlines()
+            with open(logfile, 'rb') as f:
+                return f.readlines()
 
         if isinstance(marker, str):
             marker = marker.encode('utf-8')
         data = []
         in_region = False
-        for line in open(logfile, 'rb'):
-            if in_region:
-                if line.startswith(self.markerPrefix) and marker not in line:
-                    break
-                else:
-                    data.append(line)
-            elif marker in line:
-                in_region = True
+        with open(logfile, 'rb') as f:
+            for line in f:
+                if in_region:
+                    if (line.startswith(self.markerPrefix)
+                            and marker not in line):
+                        break
+                    else:
+                        data.append(line)
+                elif marker in line:
+                    in_region = True
         return data
 
     def assertInLog(self, line, marker=None):
diff --git a/cherrypy/test/modfastcgi.py b/cherrypy/test/modfastcgi.py
index 79ec3d18..0c6d01e2 100644
--- a/cherrypy/test/modfastcgi.py
+++ b/cherrypy/test/modfastcgi.py
@@ -112,15 +112,12 @@ class ModFCGISupervisor(helper.LocalWSGISupervisor):
             fcgiconf = os.path.join(curdir, fcgiconf)
 
         # Write the Apache conf file.
-        f = open(fcgiconf, 'wb')
-        try:
+        with open(fcgiconf, 'wb') as f:
             server = repr(os.path.join(curdir, 'fastcgi.pyc'))[1:-1]
             output = self.template % {'port': self.port, 'root': curdir,
                                       'server': server}
             output = output.replace('\r\n', '\n')
             f.write(output)
-        finally:
-            f.close()
 
         result = read_process(APACHE_PATH, '-k start -f %s' % fcgiconf)
         if result:
diff --git a/cherrypy/test/modfcgid.py b/cherrypy/test/modfcgid.py
index d101bd67..ea373004 100644
--- a/cherrypy/test/modfcgid.py
+++ b/cherrypy/test/modfcgid.py
@@ -101,15 +101,12 @@ class ModFCGISupervisor(helper.LocalSupervisor):
             fcgiconf = os.path.join(curdir, fcgiconf)
 
         # Write the Apache conf file.
-        f = open(fcgiconf, 'wb')
-        try:
+        with open(fcgiconf, 'wb') as f:
             server = repr(os.path.join(curdir, 'fastcgi.pyc'))[1:-1]
             output = self.template % {'port': self.port, 'root': curdir,
                                       'server': server}
             output = ntob(output.replace('\r\n', '\n'))
             f.write(output)
-        finally:
-            f.close()
 
         result = read_process(APACHE_PATH, '-k start -f %s' % fcgiconf)
         if result:
diff --git a/cherrypy/test/modpy.py b/cherrypy/test/modpy.py
index 7c288d2c..024453e9 100644
--- a/cherrypy/test/modpy.py
+++ b/cherrypy/test/modpy.py
@@ -107,13 +107,10 @@ class ModPythonSupervisor(helper.Supervisor):
         if not os.path.isabs(mpconf):
             mpconf = os.path.join(curdir, mpconf)
 
-        f = open(mpconf, 'wb')
-        try:
+        with open(mpconf, 'wb') as f:
             f.write(self.template %
                     {'port': self.port, 'modulename': modulename,
                      'host': self.host})
-        finally:
-            f.close()
 
         result = read_process(APACHE_PATH, '-k start -f %s' % mpconf)
         if result:
diff --git a/cherrypy/test/modwsgi.py b/cherrypy/test/modwsgi.py
index da7d240b..24c72684 100644
--- a/cherrypy/test/modwsgi.py
+++ b/cherrypy/test/modwsgi.py
@@ -109,14 +109,11 @@ class ModWSGISupervisor(helper.Supervisor):
         if not os.path.isabs(mpconf):
             mpconf = os.path.join(curdir, mpconf)
 
-        f = open(mpconf, 'wb')
-        try:
+        with open(mpconf, 'wb') as f:
             output = (self.template %
                       {'port': self.port, 'testmod': modulename,
                        'curdir': curdir})
             f.write(output)
-        finally:
-            f.close()
 
         result = read_process(APACHE_PATH, '-k start -f %s' % mpconf)
         if result:
diff --git a/cherrypy/test/test_core.py b/cherrypy/test/test_core.py
index 6fde3a97..42460b3f 100644
--- a/cherrypy/test/test_core.py
+++ b/cherrypy/test/test_core.py
@@ -586,9 +586,8 @@ class CoreRequestHandlingTest(helper.CPWebCase):
     def testFavicon(self):
         # favicon.ico is served by staticfile.
         icofilename = os.path.join(localDir, '../favicon.ico')
-        icofile = open(icofilename, 'rb')
-        data = icofile.read()
-        icofile.close()
+        with open(icofilename, 'rb') as icofile:
+            data = icofile.read()
 
         self.getPage('/favicon.ico')
         self.assertBody(data)
diff --git a/cherrypy/test/test_states.py b/cherrypy/test/test_states.py
index 28dd6510..d59a4d87 100644
--- a/cherrypy/test/test_states.py
+++ b/cherrypy/test/test_states.py
@@ -424,11 +424,12 @@ test_case_name: "test_signal_handler_unsubscribe"
         p.join()
 
         # Assert the old handler ran.
-        log_lines = list(open(p.error_log, 'rb'))
-        assert any(
-            line.endswith(b'I am an old SIGTERM handler.\n')
-            for line in log_lines
-        )
+        with open(p.error_log, 'rb') as f:
+            log_lines = list(f)
+            assert any(
+                line.endswith(b'I am an old SIGTERM handler.\n')
+                for line in log_lines
+            )
 
 
 def test_safe_wait_INADDR_ANY():  # pylint: disable=invalid-name
-- 
2.35.1