Merge pull request #6216 from SylvainCorlay/comm-unregister

Unregistering comms in Comm Manager

IPython/html/static/services/kernels/js/comm.js

@@ -56,9 +56,9 b' define(['
         return comm.comm_id;
     };
     
-    CommManager.prototype.unregister_comm = function (comm_id) {
+    CommManager.prototype.unregister_comm = function (comm) {
         // Remove a comm from the mapping
-        delete this.comms[comm_id];
+        delete this.comms[comm.comm_id];
     };
     
     // comm message handlers
@@ -88,7 +88,7 b' define(['
         if (comm === undefined) {
             return;
         }
-        delete this.comms[content.comm_id];
+        this.unregister_comm(comm);
         try {
             comm.handle_close(msg);
         } catch (e) {

IPython/html/widgets/widget.py

@@ -141,7 +141,6 b' class Widget(LoggingConfigurable):'
             # Create a comm.
             self._comm = Comm(target_name=self._model_name)
             self._comm.on_msg(self._handle_msg)
-            self._comm.on_close(self._close)
             Widget.widgets[self.model_id] = self
 
             # first update
@@ -158,21 +157,18 b' class Widget(LoggingConfigurable):'
     #-------------------------------------------------------------------------
     # Methods
     #-------------------------------------------------------------------------
-    def _close(self):
-        """Private close - cleanup objects, registry entries"""
-        del Widget.widgets[self.model_id]
-        self._comm = None
 
     def close(self):
         """Close method.
 
-        Closes the widget which closes the underlying comm.
+        Closes the underlying comm.
         When the comm is closed, all of the widget views are automatically
         removed from the front-end."""
+        del Widget.widgets[self.model_id]
         if self._comm is not None:
             self._comm.close()
-            self._close()
-
+        self._comm = None
+    
     def send_state(self, key=None):
         """Sends the widget state, or a piece of it, to the front-end.
 

IPython/kernel/comm/comm.py

@@ -50,7 +50,6 b' class Comm(LoggingConfigurable):'
         if target_name:
             kwargs['target_name'] = target_name
         super(Comm, self).__init__(**kwargs)
-        get_ipython().comm_manager.register_comm(self)
         if self.primary:
             # I am primary, open my peer.
             self.open(data)
@@ -77,6 +76,8 b' class Comm(LoggingConfigurable):'
         """Open the frontend-side version of this comm"""
         if data is None:
             data = self._open_data
+        self._closed = False
+        get_ipython().comm_manager.register_comm(self)
         self._publish_msg('comm_open', data, metadata, target_name=self.target_name)
     
     def close(self, data=None, metadata=None):
@@ -87,6 +88,7 b' class Comm(LoggingConfigurable):'
         if data is None:
             data = self._close_data
         self._publish_msg('comm_close', data, metadata)
+        get_ipython().comm_manager.unregister_comm(self)
         self._closed = True
     
     def send(self, data=None, metadata=None):

IPython/kernel/comm/manager.py

@@ -72,11 +72,10 b' class CommManager(LoggingConfigurable):'
         self.comms[comm_id] = comm
         return comm_id
     
-    def unregister_comm(self, comm_id):
+    def unregister_comm(self, comm):
         """Unregister a comm, and close its counterpart"""
         # unlike get_comm, this should raise a KeyError
-        comm = self.comms.pop(comm_id)
-        comm.close()
+        comm = self.comms.pop(comm.comm_id)
     
     def get_comm(self, comm_id):
         """Get a comm with a particular id
@@ -110,13 +109,11 b' class CommManager(LoggingConfigurable):'
             self.log.error("No such comm target registered: %s", target_name)
             comm.close()
             return
-        self.register_comm(comm)
         try:
             f(comm, msg)
         except Exception:
             self.log.error("Exception opening comm with target: %s", target_name, exc_info=True)
             comm.close()
-            self.unregister_comm(comm_id)
     
     def comm_msg(self, stream, ident, msg):
         """Handler for comm_msg messages"""