Skip to content

Commit 0af2ce8

Browse files
committed
Remove ansible_mitogen Connection.close() workaround
Refs #925 #969 I'm not 100% confident that merely removing this is the full fix, without substituting something else. I am sure keeping it would be the greater of two evils. __del__() should be avoided on general principal, and it's associated with multiple intermittant CI failures, plus multiple user reported issues.
1 parent 03acf40 commit 0af2ce8

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

ansible_mitogen/connection.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,7 @@ class Connection(ansible.plugins.connection.ConnectionBase):
484484
login_context = None
485485

486486
#: Only sudo, su, and doas are supported for now.
487+
# Ansible ConnectionBase attribute, removed in Ansible >= 2.8
487488
become_methods = ['sudo', 'su', 'doas']
488489

489490
#: Dict containing init_child() return value as recorded at startup by
@@ -521,15 +522,6 @@ class Connection(ansible.plugins.connection.ConnectionBase):
521522
# set by `_get_task_vars()` for interpreter discovery
522523
_action = None
523524

524-
def __del__(self):
525-
"""
526-
Ansible cannot be trusted to always call close() e.g. the synchronize
527-
action constructs a local connection like this. So provide a destructor
528-
in the hopes of catching these cases.
529-
"""
530-
# https://github.com/dw/mitogen/issues/140
531-
self.close()
532-
533525
def on_action_run(self, task_vars, delegate_to_hostname, loader_basedir):
534526
"""
535527
Invoked by ActionModuleMixin to indicate a new task is about to start
@@ -684,6 +676,9 @@ def get_binding(self):
684676

685677
@property
686678
def connected(self):
679+
"""
680+
Ansible connection plugin property. Used by ansible-connection command.
681+
"""
687682
return self.context is not None
688683

689684
def _spec_from_via(self, proxied_inventory_name, via_spec):
@@ -842,7 +837,11 @@ def _connect(self):
842837
the _connect_*() service calls defined above to cause the master
843838
process to establish the real connection on our behalf, or return a
844839
reference to the existing one.
840+
841+
Ansible connection plugin method.
845842
"""
843+
# In some Ansible connection plugins this method returns self.
844+
# However nothing I've found uses it, it's not even assigned.
846845
if self.connected:
847846
return
848847

@@ -880,6 +879,8 @@ def close(self):
880879
Arrange for the mitogen.master.Router running in the worker to
881880
gracefully shut down, and wait for shutdown to complete. Safe to call
882881
multiple times.
882+
883+
Ansible connection plugin method.
883884
"""
884885
self._put_connection()
885886
if self.binding:
@@ -896,6 +897,8 @@ def reset(self):
896897
any local state we hold for the connection, returns the Connection to
897898
the 'disconnected' state, and informs ContextService the connection is
898899
bad somehow, and should be shut down and discarded.
900+
901+
Ansible connection plugin method.
899902
"""
900903
if self._play_context.remote_addr is None:
901904
# <2.5.6 incorrectly populate PlayContext for reset_connection
@@ -1002,6 +1005,8 @@ def exec_command(self, cmd, in_data='', sudoable=True, mitogen_chdir=None):
10021005
Data to supply on ``stdin`` of the process.
10031006
:returns:
10041007
(return code, stdout bytes, stderr bytes)
1008+
1009+
Ansible connection plugin method.
10051010
"""
10061011
emulate_tty = (not in_data and sudoable)
10071012
rc, stdout, stderr = self.get_chain().call(
@@ -1027,6 +1032,8 @@ def fetch_file(self, in_path, out_path):
10271032
Remote filesystem path to read.
10281033
:param str out_path:
10291034
Local filesystem path to write.
1035+
1036+
Ansible connection plugin method.
10301037
"""
10311038
self._connect()
10321039
ansible_mitogen.target.transfer_file(
@@ -1076,6 +1083,8 @@ def put_file(self, in_path, out_path):
10761083
Local filesystem path to read.
10771084
:param str out_path:
10781085
Remote filesystem path to write.
1086+
1087+
Ansible connection plugin method.
10791088
"""
10801089
try:
10811090
st = os.stat(in_path)

docs/changelog.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ v0.3.4.dev0
2222

2323
* :gh:issue:`929` Support Ansible 6 and ansible-core 2.13
2424
* :gh:issue:`832` Fix runtime error when using the ansible.builtin.dnf module multiple times
25+
* :gh:issue:`925` :class:`ansible_mitogen.connection.Connection` no longer tries to close the
26+
connection on destruction. This is expected to reduce cases of `mitogen.core.Error: An attempt
27+
was made to enqueue a message with a Broker that has already exitted`. However it may result in
28+
resource leaks.
29+
2530

2631
v0.3.3 (2022-06-03)
2732
-------------------

0 commit comments

Comments
 (0)