From 0909694b5080ffbc2dae3289ac5560e460bf22ea Mon Sep 17 00:00:00 2001
From: MinRK <benjaminrk@gmail.com>
Date: Wed, 13 Aug 2014 22:11:32 -0700
Subject: [PATCH] avoid race condition when deleting/starting sessions

javascript doesn't guarantee the order of AJAX requests,
so we give `Session.delete` and `Kernel.kill` a callback signature.

Changing the kernel type calls `Notebook.start_kernel`,
which terminates the previous session, if defined,
before starting the new one.

A flag is stored, to prevent multiple simultaneous attempts to start sessions, raising a SessionAlreadyStarting Error,
preventing the spec_changed event from firing.
---
 .../html/static/notebook/js/kernelselector.js | 14 +++++-
 IPython/html/static/notebook/js/menubar.js    |  7 +--
 IPython/html/static/notebook/js/notebook.js   | 44 +++++++++++++++++--
 .../html/static/services/kernels/js/kernel.js |  5 ++-
 .../static/services/sessions/js/session.js    | 25 ++++++++---
 5 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/IPython/html/static/notebook/js/kernelselector.js b/IPython/html/static/notebook/js/kernelselector.js
index 764db7273..ce9b91960 100644
--- a/IPython/html/static/notebook/js/kernelselector.js
+++ b/IPython/html/static/notebook/js/kernelselector.js
@@ -54,9 +54,19 @@ define([
             return;
         }
         var ks = this.kernelspecs[kernel_name];
+        try {
+            this.notebook.start_session(kernel_name);
+        } catch (e) {
+            if (e.name === 'SessionAlreadyStarting') {
+                console.log("Cannot change kernel while waiting for pending session start.");
+            } else {
+                // unhandled error
+                throw e;
+            }
+            // only trigger spec_changed if change was successful
+            return;
+        }
         this.events.trigger('spec_changed.Kernel', ks);
-        this.notebook.session.delete();
-        this.notebook.start_session(kernel_name);
     };
     
     KernelSelector.prototype.bind_events = function() {
diff --git a/IPython/html/static/notebook/js/menubar.js b/IPython/html/static/notebook/js/menubar.js
index a6f0b2878..67e0d5bd7 100644
--- a/IPython/html/static/notebook/js/menubar.js
+++ b/IPython/html/static/notebook/js/menubar.js
@@ -157,12 +157,13 @@ define([
             }
         });
         this.element.find('#kill_and_exit').click(function () {
-            that.notebook.session.delete();
-            setTimeout(function(){
+            var close_window = function () {
                 // allow closing of new tabs in Chromium, impossible in FF
                 window.open('', '_self', '');
                 window.close();
-            }, 500);
+            };
+            // finish with close on success or failure
+            that.notebook.session.delete(close_window, close_window);
         });
         // Edit
         this.element.find('#cut_cell').click(function () {
diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js
index 02a4439ee..64f3fbe0f 100644
--- a/IPython/html/static/notebook/js/notebook.js
+++ b/IPython/html/static/notebook/js/notebook.js
@@ -62,6 +62,7 @@ define([
         this.save_widget = options.save_widget;
         this.tooltip = new tooltip.Tooltip(this.events);
         this.ws_url = options.ws_url;
+        this._session_starting = false;
         // default_kernel_name is a temporary measure while we implement proper
         // kernel selection and delayed start. Do not rely on it.
         this.default_kernel_name = 'python';
@@ -1525,9 +1526,38 @@ define([
      * @method start_session
      */
     Notebook.prototype.start_session = function (kernel_name) {
+        var that = this;
         if (kernel_name === undefined) {
             kernel_name = this.default_kernel_name;
         }
+        if (this._session_starting) {
+            throw new session.SessionAlreadyStarting();
+        }
+        this._session_starting = true;
+        
+        if (this.session !== null) {
+            var s = this.session;
+            this.session = null;
+            // need to start the new session in a callback after delete,
+            // because javascript does not guarantee the ordering of AJAX requests (?!)
+            s.delete(function () {
+                    // on successful delete, start new session
+                    that._session_starting = false;
+                    that.start_session(kernel_name);
+                }, function (jqXHR, status, error) {
+                    // log the failed delete, but still create a new session
+                    // 404 just means it was already deleted by someone else,
+                    // but other errors are possible.
+                    utils.log_ajax_error(jqXHR, status, error);
+                    that._session_starting = false;
+                    that.start_session(kernel_name);
+                }
+            );
+            return;
+        }
+        
+        
+    
         this.session = new session.Session({
             base_url: this.base_url,
             ws_url: this.ws_url,
@@ -1539,7 +1569,10 @@ define([
             kernel_name: kernel_name,
             notebook: this});
 
-        this.session.start($.proxy(this._session_started, this));
+        this.session.start(
+            $.proxy(this._session_started, this),
+            $.proxy(this._session_start_failed, this)
+        );
     };
 
 
@@ -1548,7 +1581,8 @@ define([
      * comm manager to the widget manager
      *
      */
-    Notebook.prototype._session_started = function(){
+    Notebook.prototype._session_started = function (){
+        this._session_starting = false;
         this.kernel = this.session.kernel;
         var ncells = this.ncells();
         for (var i=0; i<ncells; i++) {
@@ -1558,7 +1592,11 @@ define([
             }
         }
     };
-    
+    Notebook.prototype._session_start_failed = function (jqxhr, status, error){
+        this._session_starting = false;
+        utils.log_ajax_error(jqxhr, status, error);
+    };
+
     /**
      * Prompt the user to restart the IPython kernel.
      * 
diff --git a/IPython/html/static/services/kernels/js/kernel.js b/IPython/html/static/services/kernels/js/kernel.js
index 322424f43..60f358304 100644
--- a/IPython/html/static/services/kernels/js/kernel.js
+++ b/IPython/html/static/services/kernels/js/kernel.js
@@ -385,13 +385,14 @@ define([
     };
 
 
-    Kernel.prototype.kill = function () {
+    Kernel.prototype.kill = function (success, faiure) {
         if (this.running) {
             this.running = false;
             var settings = {
                 cache : false,
                 type : "DELETE",
-                error : utils.log_ajax_error,
+                success : success,
+                error : error || utils.log_ajax_error,
             };
             $.ajax(utils.url_join_encode(this.kernel_url), settings);
             this.stop_channels();
diff --git a/IPython/html/static/services/sessions/js/session.js b/IPython/html/static/services/sessions/js/session.js
index c86c9754e..2c69eadd6 100644
--- a/IPython/html/static/services/sessions/js/session.js
+++ b/IPython/html/static/services/sessions/js/session.js
@@ -21,7 +21,7 @@ define([
         this.ws_url = options.ws_url;
     };
     
-    Session.prototype.start = function(callback) {
+    Session.prototype.start = function (success, error) {
         var that = this;
         var model = {
             notebook : {
@@ -40,11 +40,11 @@ define([
             dataType : "json",
             success : function (data, status, xhr) {
                 that._handle_start_success(data);
-                if (callback) {
-                    callback(data, status, xhr);
+                if (success) {
+                    success(data, status, xhr);
                 }
             },
-            error : utils.log_ajax_error,
+            error : error || utils.log_ajax_error,
         };
         var url = utils.url_join_encode(this.base_url, 'api/sessions');
         $.ajax(url, settings);
@@ -71,13 +71,14 @@ define([
         $.ajax(url, settings);
     };
     
-    Session.prototype.delete = function() {
+    Session.prototype.delete = function (success, error) {
         var settings = {
             processData : false,
             cache : false,
             type : "DELETE",
             dataType : "json",
-            error : utils.log_ajax_error,
+            success : success,
+            error : error || utils.log_ajax_error,
         };
         this.kernel.running = false;
         this.kernel.stop_channels();
@@ -119,8 +120,18 @@ define([
         this.kernel.kill();
     };
     
+    var SessionAlreadyStarting = function (message) {
+        this.name = "SessionAlreadyStarting";
+        this.message = (message || "");
+    };
+    
+    SessionAlreadyStarting.prototype = Error.prototype;
+    
     // For backwards compatability.
     IPython.Session = Session;
 
-    return {'Session': Session};
+    return {
+        Session: Session,
+        SessionAlreadyStarting: SessionAlreadyStarting,
+    };
 });