From ffc613e56bd0b4f3a6711d0fb9a6f521569571a4 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 14 Mar 2018 09:06:25 -0700 Subject: [PATCH] Trap exceptions during shutdown of comm port, fix process proxy leak PR #279 introduced some fixes to address file descriptor leaks - one of which was to shutdown the communication port. Since the kernel launcher listening on the other side may have already terminated, the shutdown method could throw an exception. This change catches, logs, then ignores such exceptions. While looking into other leaks (in this case memory), it was discovered that the process proxy instance was being leaked across kernel cycles. This change addresses that particular leak. Note: Other PRs have also been submitted to address leaks in `jupyter_client` and `notebook`. These are: [PR 360 - Fix memory leak of kernel Popen object](https://github.com/jupyter/jupyter_client/pull/360) [PR 361 - Fix memory leak of IOLoopKernelManager object](https://github.com/jupyter/jupyter_client/pull/361) [PR 3424 - Fix memory leak of iopub object in activity monitoring](https://github.com/jupyter/notebook/pull/3424) --- enterprise_gateway/services/kernels/remotemanager.py | 1 + .../services/processproxies/processproxy.py | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/enterprise_gateway/services/kernels/remotemanager.py b/enterprise_gateway/services/kernels/remotemanager.py index 83420670d..5032bc507 100644 --- a/enterprise_gateway/services/kernels/remotemanager.py +++ b/enterprise_gateway/services/kernels/remotemanager.py @@ -169,6 +169,7 @@ def cleanup(self, connection_file=True): # which then prevents process proxy cleanup. if self.process_proxy: self.process_proxy.cleanup() + self.process_proxy = None return super(RemoteKernelManager, self).cleanup(connection_file) def get_connection_info(self, session=False): diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index 87764acf1..3a3476a9a 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -26,7 +26,6 @@ from Crypto.Cipher import AES - # Default logging level of paramiko produces too much noise - raise to warning only. logging.getLogger('paramiko').setLevel(os.getenv('EG_SSH_LOG_LEVEL', logging.WARNING)) @@ -701,7 +700,12 @@ def shutdown_listener(self): "(using remote kill): {}".format(self.comm_ip, self.comm_port, self.kernel_id, str(e))) finally: - sock.shutdown(SHUT_WR) + try: + sock.shutdown(SHUT_WR) + except Exception as e2: + self.log.warning("Exception occurred attempting to shutdown communication socket to {}:{} " + "for KernelID '{}' (ignored): {}".format(self.comm_ip, self.comm_port, + self.kernel_id, str(e2))) sock.close() # Also terminate the tunnel process for the communication port - if in play. Failure to terminate