Merge pull request #3485 from jasongrout/throttle

Fix message throttle by making a copy of the callbacks dict
jupyter-widgets · Jun 21, 2022 · e6ba2b6 · e6ba2b6
2 parents d2f2bdf + 353316a
commit e6ba2b6
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 14 deletions.
diff --git a/packages/base/src/widget.ts b/packages/base/src/widget.ts
@@ -334,9 +334,10 @@ export class WidgetModel extends Backbone.Model {
     if (this.comm !== void 0) {
       if (msg.content.execution_state === 'idle') {
         this._pending_msgs--;
+        // Sanity check for logic errors that may push this below zero.
         if (this._pending_msgs < 0) {
           console.error(
-            `Pending messages < 0 (=${this._pending_msgs}), which is unexpected`
+            `Jupyter Widgets message throttle: Pending messages < 0 (=${this._pending_msgs}), which is unexpected. Resetting to 0 to continue.`
           );
           this._pending_msgs = 0; // do not break message throttling in case of unexpected errors
         }
@@ -553,17 +554,18 @@ export class WidgetModel extends Backbone.Model {
       return '';
     }
     try {
-      callbacks.iopub = callbacks.iopub || {};
-      if (callbacks.iopub.previouslyUsedByJupyterWidgets === undefined) {
-        // Do not break other code that also wants to listen to status updates
-        callbacks.iopub.statusPrevious = callbacks.iopub.status;
-      }
-      // else callbacks.iopub.status is the old handler, that we should not reuse
-      callbacks.iopub.previouslyUsedByJupyterWidgets = true;
+      // Make a 2-deep copy so we don't modify the caller's callbacks object.
+      callbacks = {
+        shell: { ...callbacks.shell },
+        iopub: { ...callbacks.iopub },
+        input: callbacks.input,
+      };
+      // Save the caller's status callback so we can call it after we handle the message.
+      const statuscb = callbacks.iopub.status;
       callbacks.iopub.status = (msg: KernelMessage.IStatusMsg): void => {
         this._handle_status(msg);
-        if (callbacks.iopub.statusPrevious) {
-          callbacks.iopub.statusPrevious(msg);
+        if (statuscb) {
+          statuscb(msg);
         }
       };
 

diff --git a/packages/base/test/src/widget_test.ts b/packages/base/test/src/widget_test.ts
@@ -643,7 +643,7 @@ describe('WidgetModel', function () {
       });
       expect(this.widget._msg_buffer_callbacks).to.not.equals(null);
       // have the comm send a status idle message
-      callbacks.iopub.status({
+      this.comm.send.lastCall.args[1].iopub.status({
         content: {
           execution_state: 'idle',
         },
@@ -662,7 +662,7 @@ describe('WidgetModel', function () {
         },
         buffer_paths: [],
       });
-      callbacks.iopub.status({
+      this.comm.send.lastCall.args[1].iopub.status({
         content: {
           execution_state: 'idle',
         },
@@ -687,7 +687,7 @@ describe('WidgetModel', function () {
         state: { a: 'sync test - 2' },
         buffer_paths: [],
       });
-      callbacks.iopub.status({
+      this.comm.send.lastCall.args[1].iopub.status({
         content: {
           execution_state: 'idle',
         },
@@ -703,7 +703,7 @@ describe('WidgetModel', function () {
         },
         buffer_paths: [],
       });
-      callbacks.iopub.status({
+      this.comm.send.lastCall.args[1].iopub.status({
         content: {
           execution_state: 'idle',
         },