Skip to content

Commit 367b0bd

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 a4ac357 commit 367b0bd

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
@@ -219,8 +219,20 @@ def pull_file(self, remote_source, local_dir):
219219

220220
def close(self) -> None:
221221
if self._is_connected():
222+
transport = self.ssh_client.get_transport()
222223
self.ssh_client.close()
223224

225+
# ssh_client.close calls transport.close, but transport.close does
226+
# not always wait for the transport thread to be stopped. See impl
227+
# of Transport.close in paramiko and issue
228+
# https://github.com/paramiko/paramiko/issues/520
229+
logger.debug("Waiting for transport thread to stop")
230+
transport.join(30)
231+
if transport.is_alive():
232+
logger.warning("SSH transport thread did not shut down")
233+
else:
234+
logger.debug("SSH transport thread stopped")
235+
224236
def isdir(self, path):
225237
"""Return true if the path refers to an existing directory.
226238

parsl/tests/test_providers/test_local_provider.py

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

0 commit comments

Comments
 (0)