Skip to content

Commit 3e007d8

Browse files
committed
close in test... but doesn't close in DFK ... so that needs to happen too (and be tested)
(see different patch) it should be the responsibility of the DFK to close channels (at least in that other patch) and so that's why this test, which avoids the DFK, needs to close channels itself but looks like channel.close() doesn't close down threads enough? I can see it now closes the transport - the transport thread is now reported as unconnected, prior to calling close it was reporting itself as connected. but how to make the transport thread go away entirely? it looks like the thread goes away "eventually" - a 20s sleep for example, is long enough - so how to do that with joins?
1 parent 1f6c646 commit 3e007d8

File tree

2 files changed

+23
-7
lines changed

2 files changed

+23
-7
lines changed

parsl/channels/ssh/ssh.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,20 @@ def pull_file(self, remote_source, local_dir):
227227

228228
def close(self) -> None:
229229
if self._is_connected():
230+
transport = self.ssh_client.get_transport()
230231
self.ssh_client.close()
231232

233+
# ssh_client.close calls transport.close, but transport.close does
234+
# not always wait for the transport thread to be stopped. See impl
235+
# of Transport.close in paramiko and issue
236+
# https://github.com/paramiko/paramiko/issues/520
237+
logger.debug("Waiting for transport thread to stop")
238+
transport.join(30)
239+
if transport.is_alive():
240+
logger.warning("SSH transport thread did not shut down")
241+
else:
242+
logger.debug("SSH transport thread stopped")
243+
232244
def isdir(self, path):
233245
"""Return true if the path refers to an existing directory.
234246

parsl/tests/test_providers/test_local_provider.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,17 @@ def test_ssh_channel():
9292
# already exist, so create it here.
9393
pathlib.Path('{}/known.hosts'.format(config_dir)).touch(mode=0o600)
9494
script_dir = tempfile.mkdtemp()
95-
p = LocalProvider(channel=SSHChannel('127.0.0.1', port=server_port,
96-
script_dir=remote_script_dir,
97-
host_keys_filename='{}/known.hosts'.format(config_dir),
98-
key_filename=priv_key),
99-
launcher=SingleNodeLauncher(debug=False))
100-
p.script_dir = script_dir
101-
_run_tests(p)
95+
channel = SSHChannel('127.0.0.1', port=server_port,
96+
script_dir=remote_script_dir,
97+
host_keys_filename='{}/known.hosts'.format(config_dir),
98+
key_filename=priv_key)
99+
try:
100+
p = LocalProvider(channel=channel,
101+
launcher=SingleNodeLauncher(debug=False))
102+
p.script_dir = script_dir
103+
_run_tests(p)
104+
finally:
105+
channel.close()
102106
finally:
103107
_stop_sshd(sshd_thread)
104108

0 commit comments

Comments
 (0)