-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-2433 Fix Python 3 ServerDescription/Exception memory leak #520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice test. What is behind the choice of using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5 is arbitrary but it allows for some tolerance for race conditions, like having several clients open in the background (creating and destroying ServerDescriptions). |
||
|
||
|
||
class TestExhaustCursor(IntegrationTest): | ||
"""Test that clients properly handle errors from exhaust cursors.""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason you are purging the traceback, etc. in this location as opposed to after emitting the heartbeat failed event in
Monitor._check_server
? Why preclude any exception on a ServerDescription from having a traceback?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is a bit odd to mutate the exception in the ServerDescription constructor. I moved this logic to the Monitor.