Skip to content

Commit e6ba2b6

Browse files
authored
Merge pull request #3485 from jasongrout/throttle
Fix message throttle by making a copy of the callbacks dict
2 parents d2f2bdf + 353316a commit e6ba2b6

File tree

2 files changed

+16
-14
lines changed

2 files changed

+16
-14
lines changed

packages/base/src/widget.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,10 @@ export class WidgetModel extends Backbone.Model {
334334
if (this.comm !== void 0) {
335335
if (msg.content.execution_state === 'idle') {
336336
this._pending_msgs--;
337+
// Sanity check for logic errors that may push this below zero.
337338
if (this._pending_msgs < 0) {
338339
console.error(
339-
`Pending messages < 0 (=${this._pending_msgs}), which is unexpected`
340+
`Jupyter Widgets message throttle: Pending messages < 0 (=${this._pending_msgs}), which is unexpected. Resetting to 0 to continue.`
340341
);
341342
this._pending_msgs = 0; // do not break message throttling in case of unexpected errors
342343
}
@@ -553,17 +554,18 @@ export class WidgetModel extends Backbone.Model {
553554
return '';
554555
}
555556
try {
556-
callbacks.iopub = callbacks.iopub || {};
557-
if (callbacks.iopub.previouslyUsedByJupyterWidgets === undefined) {
558-
// Do not break other code that also wants to listen to status updates
559-
callbacks.iopub.statusPrevious = callbacks.iopub.status;
560-
}
561-
// else callbacks.iopub.status is the old handler, that we should not reuse
562-
callbacks.iopub.previouslyUsedByJupyterWidgets = true;
557+
// Make a 2-deep copy so we don't modify the caller's callbacks object.
558+
callbacks = {
559+
shell: { ...callbacks.shell },
560+
iopub: { ...callbacks.iopub },
561+
input: callbacks.input,
562+
};
563+
// Save the caller's status callback so we can call it after we handle the message.
564+
const statuscb = callbacks.iopub.status;
563565
callbacks.iopub.status = (msg: KernelMessage.IStatusMsg): void => {
564566
this._handle_status(msg);
565-
if (callbacks.iopub.statusPrevious) {
566-
callbacks.iopub.statusPrevious(msg);
567+
if (statuscb) {
568+
statuscb(msg);
567569
}
568570
};
569571

packages/base/test/src/widget_test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ describe('WidgetModel', function () {
643643
});
644644
expect(this.widget._msg_buffer_callbacks).to.not.equals(null);
645645
// have the comm send a status idle message
646-
callbacks.iopub.status({
646+
this.comm.send.lastCall.args[1].iopub.status({
647647
content: {
648648
execution_state: 'idle',
649649
},
@@ -662,7 +662,7 @@ describe('WidgetModel', function () {
662662
},
663663
buffer_paths: [],
664664
});
665-
callbacks.iopub.status({
665+
this.comm.send.lastCall.args[1].iopub.status({
666666
content: {
667667
execution_state: 'idle',
668668
},
@@ -687,7 +687,7 @@ describe('WidgetModel', function () {
687687
state: { a: 'sync test - 2' },
688688
buffer_paths: [],
689689
});
690-
callbacks.iopub.status({
690+
this.comm.send.lastCall.args[1].iopub.status({
691691
content: {
692692
execution_state: 'idle',
693693
},
@@ -703,7 +703,7 @@ describe('WidgetModel', function () {
703703
},
704704
buffer_paths: [],
705705
});
706-
callbacks.iopub.status({
706+
this.comm.send.lastCall.args[1].iopub.status({
707707
content: {
708708
execution_state: 'idle',
709709
},

0 commit comments

Comments
 (0)