Skip to content
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

Fix infinite loop in gateway client when kernel no longer exists #5678

Open
wants to merge 1 commit into
base: 6.4.x
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions notebook/gateway/handlers.py
Expand Up @@ -12,7 +12,7 @@
from tornado.concurrent import Future
from tornado.ioloop import IOLoop, PeriodicCallback
from tornado.websocket import WebSocketHandler, websocket_connect
from tornado.httpclient import HTTPRequest
from tornado.httpclient import HTTPRequest, HTTPClientError
from tornado.escape import url_escape, json_decode, utf8

from ipython_genutils.py3compat import cast_unicode
Expand Down Expand Up @@ -153,10 +153,16 @@ def _connect(self, kernel_id):
self.ws_future.add_done_callback(self._connection_done)

def _connection_done(self, fut):
if not self.disconnected and fut.exception() is None: # prevent concurrent.futures._base.CancelledError
exc = fut.exception()
if not self.disconnected and exc is None: # prevent concurrent.futures._base.CancelledError
self.ws = fut.result()
self.log.debug("Connection is ready: ws: {}".format(self.ws))
else:
if exc is not None and isinstance(exc, HTTPClientError):
self.log.warning("Encountered this HTTPError: {}. Disconnecting if it's a 404.".format(exc))
if exc.code == 404:
# Disconnect if the kernel no longer exists
self._disconnect()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commenting here since github won't let me comment outside edited lines. Basically, this kind of logic could maybe go on line 200, or I could add exponential backoff there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or I could add exponential backoff there

Could you please add this?

self.log.warning("Websocket connection has been closed via client disconnect or due to error. "
"Kernel with ID '{}' may not be terminated on GatewayClient: {}".
format(self.kernel_id, GatewayClient.instance().url))
Expand Down