From 0e63ebb1db0946bc667b7a910f9d2f6fccd45151 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Fri, 16 Nov 2018 08:43:23 -0800 Subject: [PATCH] Update session_exists() to account for invalid sessions due to culling When kernels are culled, the kernel is terminated in the background, unbeknownst to the session management. As a result, invalid sessions can be produced that appear to exist, yet cannot produce a model from the persisted row due to the associated kernel no longer being active. Prior to this change, these sessions, when encountered via a subsequent call to `get_session()`, would be deleted and a KeyError would be raised. This change updates the existence check to tolerate those kinds of sessions. It removes such sessions (as would happen previously), but rather than raise a KeyError when attempting to convert the row to a dictionary, it logs a warning and returns None, which then allows `session_exists()` to return False since the session was removed (as was ultimately the case previously). Calls to `get_session()` remain just as before and have the potential to raise `KeyError` in such cases. The difference now being that the `KeyError` is accompanied by a message indicating the cause. Fixes #4209 --- notebook/services/sessions/sessionmanager.py | 29 ++++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/notebook/services/sessions/sessionmanager.py b/notebook/services/sessions/sessionmanager.py index f01be13550..ee70eb0810 100644 --- a/notebook/services/sessions/sessionmanager.py +++ b/notebook/services/sessions/sessionmanager.py @@ -59,10 +59,17 @@ def __del__(self): def session_exists(self, path): """Check to see if the session of a given name exists""" self.cursor.execute("SELECT * FROM session WHERE path=?", (path,)) - reply = self.cursor.fetchone() - if reply is None: + row = self.cursor.fetchone() + if row is None: return False else: + # Note, although we found a row for the session, the associated kernel may have + # been culled or died unexpectedly. If that's the case, we should delete the + # row, thereby terminating the session. This can be done via a call to + # row_to_model that tolerates that condition. If row_to_model returns None, + # we'll return false, since, at that point, the session doesn't exist anyway. + if self.row_to_model(row, tolerate_culled=True) is None: + return False return True def new_session_id(self): @@ -198,15 +205,25 @@ def update_session(self, session_id, **kwargs): query = "UPDATE session SET %s WHERE session_id=?" % (', '.join(sets)) self.cursor.execute(query, list(kwargs.values()) + [session_id]) - def row_to_model(self, row): + def row_to_model(self, row, tolerate_culled=False): """Takes sqlite database session row and turns it into a dictionary""" if row['kernel_id'] not in self.kernel_manager: - # The kernel was killed or died without deleting the session. + # The kernel was culled or died without deleting the session. # We can't use delete_session here because that tries to find - # and shut down the kernel. + # and shut down the kernel - so we'll delete the row directly. + # + # If caller wishes to tolerate culled kernels, log a warning + # and return None. Otherwise, raise KeyError with a similar + # message. self.cursor.execute("DELETE FROM session WHERE session_id=?", (row['session_id'],)) - raise KeyError + msg = "Kernel '{kernel_id}' appears to have been culled or died unexpectedly, " \ + "invalidating session '{session_id}'. The session has been removed.".\ + format(kernel_id=row['kernel_id'],session_id=row['session_id']) + if tolerate_culled: + self.log.warning(msg + " Continuing...") + return None + raise KeyError(msg) model = { 'id': row['session_id'],