From 93c6542edc2d46eb754fa71d0216133b6d83ad5c Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 13 Oct 2022 10:43:38 -0500 Subject: [PATCH 1/3] Run the leakchecks on Python 3.11 --- .github/workflows/ci.yml | 5 ++--- src/gevent/resolver/__init__.py | 9 +++++++++ src/gevent/resolver/ares.py | 15 ++++++++++++++- src/gevent/resolver/cares.pyx | 8 ++++---- src/gevent/tests/test__ares_timeout.py | 1 + 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1a3d3466..02ac63678 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -350,11 +350,10 @@ jobs: run: | python -mgevent.tests --second-chance $G_USE_COV --ignore tests_that_dont_use_resolver.txt - name: "Tests: leakchecks" - # Run the leaktests; this seems to be extremely slow on Python 3.7 - # XXX: Figure out why. Can we reproduce locally? + # Run the leaktests; # This is incredibly important and we MUST have an environment that successfully passes # these tests. - if: matrix.python-version == 2.7 && startsWith(runner.os, 'Linux') + if: (matrix.python-version == 2.7 || startsWith(matrix.python-version, '3.11')) && startsWith(runner.os, 'Linux') env: GEVENTTEST_LEAKCHECK: 1 run: | diff --git a/src/gevent/resolver/__init__.py b/src/gevent/resolver/__init__.py index 0526f0ac7..2a2ffd5ff 100644 --- a/src/gevent/resolver/__init__.py +++ b/src/gevent/resolver/__init__.py @@ -135,6 +135,15 @@ class AbstractResolver(object): and k not in ('SOCK_CLOEXEC', 'SOCK_MAX_SIZE') } + def close(self): + """ + Release resources held by this object. + + Subclasses that define resources should override. + + .. versionadded:: NEXT + """ + @staticmethod def fixup_gaierror(func): import functools diff --git a/src/gevent/resolver/ares.py b/src/gevent/resolver/ares.py index b5d1dab74..ae99b899a 100644 --- a/src/gevent/resolver/ares.py +++ b/src/gevent/resolver/ares.py @@ -4,6 +4,7 @@ """ from __future__ import absolute_import, print_function, division import os +import warnings from _socket import gaierror from _socket import herror @@ -116,12 +117,17 @@ class Resolver(AbstractResolver): Handling of localhost and broadcast names is now more consistent. + .. versionchanged:: NEXT + Now has a ``__del__`` method that warns if the object is destroyed + without being properly closed. + .. _c-ares: http://c-ares.haxx.se """ cares_class = channel def __init__(self, hub=None, use_environ=True, **kwargs): + AbstractResolver.__init__(self) if hub is None: hub = get_hub() self.hub = hub @@ -134,7 +140,7 @@ def __init__(self, hub=None, use_environ=True, **kwargs): self.cares = self.cares_class(hub.loop, **kwargs) self.pid = os.getpid() self.params = kwargs - self.fork_watcher = hub.loop.fork(ref=False) + self.fork_watcher = hub.loop.fork(ref=False) # We shouldn't keep the loop alive self.fork_watcher.start(self._on_fork) def __repr__(self): @@ -149,11 +155,18 @@ def _on_fork(self): self.pid = pid def close(self): + AbstractResolver.close(self) if self.cares is not None: self.hub.loop.run_callback(self.cares.destroy) self.cares = None self.fork_watcher.stop() + def __del__(self): + if self.cares is not None: + warnings.warn("cares Resolver destroyed while not closed", + ResourceWarning) + self.close() + def _gethostbyname_ex(self, hostname_bytes, family): while True: ares = self.cares diff --git a/src/gevent/resolver/cares.pyx b/src/gevent/resolver/cares.pyx index c84f87e7f..24f261a16 100644 --- a/src/gevent/resolver/cares.pyx +++ b/src/gevent/resolver/cares.pyx @@ -412,6 +412,9 @@ cdef class channel: return '<%s at 0x%x _timer=%r _watchers[%s]>' % args def destroy(self): + self.__destroy() + + cdef __destroy(self): if self.channel: # XXX ares_library_cleanup? cares.ares_destroy(self.channel) @@ -421,10 +424,7 @@ cdef class channel: self.loop = None def __dealloc__(self): - if self.channel: - # XXX ares_library_cleanup? - cares.ares_destroy(self.channel) - self.channel = NULL + self.__destroy() cpdef set_servers(self, servers=None): if not self.channel: diff --git a/src/gevent/tests/test__ares_timeout.py b/src/gevent/tests/test__ares_timeout.py index cc9ed8549..cb88ff2e0 100644 --- a/src/gevent/tests/test__ares_timeout.py +++ b/src/gevent/tests/test__ares_timeout.py @@ -34,6 +34,7 @@ def reader(): r = Resolver(servers=[address[0]], timeout=0.001, tries=1, udp_port=address[-1]) + self._close_on_teardown(r) with self.assertRaisesRegex(socket.herror, "ARES_ETIMEOUT"): r.gethostbyname('www.google.com') From b45460ccbafc77d07ae161de8a7b5dbaa5c1783c Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 13 Oct 2022 12:33:48 -0500 Subject: [PATCH 2/3] Stop ignoring FrameType and TracebackType during leakchecks. They're important objects. Locally, the 3.11 tests pass fine this way. The 2.7 tests had two failures, one from being unable to make a connection in test__server, and one from a timeout in test__doctest. I suspect those are unrelated; lets see what CI says. --- src/gevent/testing/leakcheck.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/gevent/testing/leakcheck.py b/src/gevent/testing/leakcheck.py index be70e009a..1c8a1fe57 100644 --- a/src/gevent/testing/leakcheck.py +++ b/src/gevent/testing/leakcheck.py @@ -21,7 +21,6 @@ import sys import gc -import types from functools import wraps import unittest @@ -52,8 +51,10 @@ class and specify variants of behaviour (such as pool sizes). class _RefCountChecker(object): - # Some builtin things that we ignore - IGNORED_TYPES = (tuple, dict, types.FrameType, types.TracebackType) + # Some builtin things that we ignore. + # For awhile, we also ignored types.FrameType and types.TracebackType, + # but those are important and often involved in leaks. + IGNORED_TYPES = (tuple, dict,) try: CALLBACK_KIND = gevent.core.callback except AttributeError: From ea8073e8918564b4353f5893be9b127ae9145230 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 13 Oct 2022 16:07:04 -0500 Subject: [PATCH 3/3] Simplified according to pylint. --- src/gevent/tests/test__environ.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gevent/tests/test__environ.py b/src/gevent/tests/test__environ.py index a4cfce438..489d158e8 100644 --- a/src/gevent/tests/test__environ.py +++ b/src/gevent/tests/test__environ.py @@ -4,7 +4,7 @@ import gevent.core import subprocess -if sys.argv[1:] == []: +if not sys.argv[1:]: os.environ['GEVENT_BACKEND'] = 'select' # (not in Py2) pylint:disable=consider-using-with popen = subprocess.Popen([sys.executable, __file__, '1'])