You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
417 lines
15 KiB
417 lines
15 KiB
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
|
|
|