From 1b40b9417de22fff3b76d087356397e62874fa01 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Wed, 18 Nov 2020 22:19:21 -0800 Subject: [PATCH 1/2] PYTHON-2433 Fix Python 3 ServerDescription/Exception memory leak When the SDAM monitor check fails, a ServerDescription is created from the exception. This exception is kept alive via the ServerDescription.error field. Unfortunately, the exception's traceback contains a reference to the previous ServerDescription. Altogether this means that each consecutively failing check leaks memory by building an ever growing chain of ServerDescription -> Exception -> Traceback -> Frame -> ServerDescription -> ... objects. This change breaks the chain and prevents the memory leak by clearing the Exception's __traceback__, __context__, and __cause__ fields. --- pymongo/server_description.py | 6 ++++++ test/test_client.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/pymongo/server_description.py b/pymongo/server_description.py index 4a1fe38604..eb81fd5304 100644 --- a/pymongo/server_description.py +++ b/pymongo/server_description.py @@ -15,6 +15,7 @@ """Represent one server the driver is connected to.""" from bson import EPOCH_NAIVE +from bson.py3compat import PY3 from pymongo.server_type import SERVER_TYPE from pymongo.ismaster import IsMaster from pymongo.monotonic import time as _time @@ -69,6 +70,11 @@ def __init__( self._me = ismaster.me self._last_update_time = _time() self._error = error + # PYTHON-2433 Clear error traceback info. + if error and PY3: + error.__traceback__ = None + error.__context__ = None + error.__cause__ = None self._topology_version = ismaster.topology_version if error: if hasattr(error, 'details') and isinstance(error.details, dict): diff --git a/test/test_client.py b/test/test_client.py index 19ea1375c2..dc21b357fc 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -57,6 +57,7 @@ from pymongo.driver_info import DriverInfo from pymongo.pool import SocketInfo, _METADATA from pymongo.read_preferences import ReadPreference +from pymongo.server_description import ServerDescription from pymongo.server_selectors import (any_server_selector, writable_server_selector) from pymongo.server_type import SERVER_TYPE @@ -1614,6 +1615,33 @@ def test_direct_connection(self): with self.assertRaises(ConfigurationError): MongoClient(['host1', 'host2'], directConnection=True) + def test_continuous_network_errors(self): + def server_description_count(): + i = 0 + for obj in gc.get_objects(): + try: + if isinstance(obj, ServerDescription): + i += 1 + except ReferenceError: + pass + return i + gc.collect() + with client_knobs(min_heartbeat_interval=0.003): + client = MongoClient( + 'invalid:27017', + heartbeatFrequencyMS=3, + serverSelectionTimeoutMS=100) + initial_count = server_description_count() + self.addCleanup(client.close) + with self.assertRaises(ServerSelectionTimeoutError): + client.test.test.find_one() + gc.collect() + final_count = server_description_count() + # If a bug like PYTHON-2433 is reintroduced then too many + # ServerDescriptions will be kept alive and this test will fail: + # AssertionError: 4 != 22 within 5 delta (18 difference) + self.assertAlmostEqual(initial_count, final_count, delta=5) + class TestExhaustCursor(IntegrationTest): """Test that clients properly handle errors from exhaust cursors.""" From 57f7a7eb276e25d9fed775d188248baba5ec7003 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Fri, 20 Nov 2020 16:27:38 -0800 Subject: [PATCH 2/2] PYTHON-2433 Refactor to sanitize error in Monitor --- pymongo/monitor.py | 12 ++++++++++++ pymongo/server_description.py | 6 ------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pymongo/monitor.py b/pymongo/monitor.py index 81502591bf..27f9bbb992 100644 --- a/pymongo/monitor.py +++ b/pymongo/monitor.py @@ -18,6 +18,8 @@ import threading import weakref +from bson.py3compat import PY3 + from pymongo import common, periodic_executor from pymongo.errors import (NotMasterError, OperationFailure, @@ -30,6 +32,14 @@ from pymongo.srv_resolver import _SrvResolver +def _sanitize(error): + """PYTHON-2433 Clear error traceback info.""" + if PY3: + error.__traceback__ = None + error.__context__ = None + error.__cause__ = None + + class MonitorBase(object): def __init__(self, topology, name, interval, min_interval): """Base class to do periodic work on a background thread. @@ -169,6 +179,7 @@ def _run(self): try: self._server_description = self._check_server() except _OperationCancelled as exc: + _sanitize(exc) # Already closed the connection, wait for the next check. self._server_description = ServerDescription( self._server_description.address, error=exc) @@ -212,6 +223,7 @@ def _check_server(self): except ReferenceError: raise except Exception as error: + _sanitize(error) sd = self._server_description address = sd.address duration = _time() - start diff --git a/pymongo/server_description.py b/pymongo/server_description.py index eb81fd5304..4a1fe38604 100644 --- a/pymongo/server_description.py +++ b/pymongo/server_description.py @@ -15,7 +15,6 @@ """Represent one server the driver is connected to.""" from bson import EPOCH_NAIVE -from bson.py3compat import PY3 from pymongo.server_type import SERVER_TYPE from pymongo.ismaster import IsMaster from pymongo.monotonic import time as _time @@ -70,11 +69,6 @@ def __init__( self._me = ismaster.me self._last_update_time = _time() self._error = error - # PYTHON-2433 Clear error traceback info. - if error and PY3: - error.__traceback__ = None - error.__context__ = None - error.__cause__ = None self._topology_version = ismaster.topology_version if error: if hasattr(error, 'details') and isinstance(error.details, dict):