Fix frontends getting out of sync.

As described in #3111 it can happen that a frontend can get out of sync
due to not echoing back messages from the frontend.

Widgets can opt out of this behaviour if it is too costly and/or does
not make sense, e.g. the data from the file widget.

Fixes #3111

avoid echo/jitter in frontend

put jupyter echo behaviour behind a flag

minimize diff

remove spacing

prettier

lint

fix: handle messages correctly when echoing disabled

minimize diff

minimize diff

fix and test

fix test, do not use set (no determinstic order)

fix: echos from other clients are not unexpected

comment and reformat

better variable name

Work-in-progress update for echo updates to have their own key in update messages

Update protocol description

Update js echo logic to handle echo_state key

Starts fixing #3392

WIP consolidating logic for property lock.

WIP simple reflection immediately on setting state.

WIP Remove code about sending echo messages combined with any kernel updates

Remove comments and simplify so the final diff is simpler

Add debug test runner

Experiment making echo updates a separate message

Bump widget protocol version number back to 2.1.0

Update no_echo metadata name to echo_update for clarity.

Having a negative in the name was confusing to me, so I renamed the property echo_update. Set it to False to disable echos for that attribute.

packages/base-manager/test/src/manager_test.ts

@@ -168,7 +168,7 @@ describe('ManagerBase', function () {
           },
         },
         metadata: {
-          version: '2.0.0',
+          version: '2.1.0',
         },
       });
       expect(model.comm).to.equal(comm);
@@ -231,7 +231,7 @@ describe('ManagerBase', function () {
         },
         buffers: [new DataView(new Uint8Array([1, 2, 3]).buffer)],
         metadata: {
-          version: '2.0.0',
+          version: '2.1.0',
         },
       });
       expect(model.comm).to.equal(comm);

packages/base/package.json

@@ -26,6 +26,7 @@
     "test:coverage": "npm run build:test && webpack --config test/webpack-cov.conf.js && karma start test/karma-cov.conf.js",
     "test:unit": "npm run test:unit:firefox && npm run test:unit:chrome",
     "test:unit:chrome": "npm run test:unit:default -- --browsers=Chrome",
+    "test:unit:chrome:debug": "npm run test:unit:default -- --browsers=Chrome --single-run=false",
     "test:unit:default": "npm run build:test && karma start test/karma.conf.js --log-level debug",
     "test:unit:firefox": "npm run test:unit:default -- --browsers=Firefox",
     "test:unit:firefox:headless": "npm run test:unit:default -- --browsers=FirefoxHeadless",

packages/base/src/version.ts

@@ -3,4 +3,4 @@
 
 export const JUPYTER_WIDGETS_VERSION = '2.0.0';
 
-export const PROTOCOL_VERSION = '2.0.0';
+export const PROTOCOL_VERSION = '2.1.0';

packages/base/src/widget.ts

@@ -115,6 +115,9 @@ export class WidgetModel extends Backbone.Model {
     attributes: Backbone.ObjectHash,
     options: IBackboneModelOptions
   ): void {
+    this.expectedEchoMsgIds = new Map<string, string>();
+    this.attrsToUpdate = new Set<string>();
+
     super.initialize(attributes, options);
 
     // Attributes should be initialized here, since user initialization may depend on it
@@ -221,13 +224,46 @@ export class WidgetModel extends Backbone.Model {
     const method = data.method;
     switch (method) {
       case 'update':
+      case 'echo_update':
         this.state_change = this.state_change
           .then(() => {
             const state = data.state;
-            const buffer_paths = data.buffer_paths || [];
-            const buffers = msg.buffers || [];
+            const buffer_paths = data.buffer_paths ?? [];
+            const buffers = msg.buffers?.slice(0, buffer_paths.length) ?? [];
             utils.put_buffers(state, buffer_paths, buffers);
+
+            if (msg.parent_header && method === 'echo_update') {
+              const msgId = (msg.parent_header as any).msg_id;
+              // we may have echos coming from other clients, we only care about
+              // dropping echos for which we expected a reply
+              const expectedEcho = Object.keys(state).filter((attrName) =>
+                this.expectedEchoMsgIds.has(attrName)
+              );
+              expectedEcho.forEach((attrName: string) => {
+                // Skip echo messages until we get the reply we are expecting.
+                const isOldMessage =
+                  this.expectedEchoMsgIds.get(attrName) !== msgId;
+                if (isOldMessage) {
+                  // Ignore an echo update that comes before our echo.
+                  delete state[attrName];
+                } else {
+                  // we got our echo confirmation, so stop looking for it
+                  this.expectedEchoMsgIds.delete(attrName);
+                  // Start accepting echo updates unless we plan to send out a new state soon
+                  if (
+                    this._msg_buffer !== null &&
+                    Object.prototype.hasOwnProperty.call(
+                      this._msg_buffer,
+                      attrName
+                    )
+                  ) {
+                    delete state[attrName];
+                  }
+                }
+              });
+            }
             return (this.constructor as typeof WidgetModel)._deserialize_state(
+              // Combine the state updates, with preference for kernel updates
               state,
               this.widget_manager
             );
@@ -300,7 +336,11 @@ export class WidgetModel extends Backbone.Model {
         this._pending_msgs--;
         // Send buffer if one is waiting and we are below the throttle.
         if (this._msg_buffer !== null && this._pending_msgs < 1) {
-          this.send_sync_message(this._msg_buffer, this._msg_buffer_callbacks);
+          const msgId = this.send_sync_message(
+            this._msg_buffer,
+            this._msg_buffer_callbacks
+          );
+          this.rememberLastUpdateFor(msgId);
           this._msg_buffer = null;
           this._msg_buffer_callbacks = null;
         }
@@ -415,6 +455,10 @@ export class WidgetModel extends Backbone.Model {
       }
     }
 
+    Object.keys(attrs).forEach((attrName: string) => {
+      this.attrsToUpdate.add(attrName);
+    });
+
     const msgState = this.serialize(attrs);
 
     if (Object.keys(msgState).length > 0) {
@@ -444,7 +488,8 @@ export class WidgetModel extends Backbone.Model {
       } else {
         // We haven't exceeded the throttle, send the message like
         // normal.
-        this.send_sync_message(attrs, callbacks);
+        const msgId = this.send_sync_message(attrs, callbacks);
+        this.rememberLastUpdateFor(msgId);
         // Since the comm is a one-way communication, assume the message
         // arrived and was processed successfully.
         // Don't call options.success since we don't have a model back from
@@ -453,6 +498,12 @@ export class WidgetModel extends Backbone.Model {
       }
     }
   }
+  rememberLastUpdateFor(msgId: string) {
+    this.attrsToUpdate.forEach((attrName) => {
+      this.expectedEchoMsgIds.set(attrName, msgId);
+    });
+    this.attrsToUpdate = new Set<string>();
+  }
 
   /**
    * Serialize widget state.
@@ -488,9 +539,9 @@ export class WidgetModel extends Backbone.Model {
   /**
    * Send a sync message to the kernel.
    */
-  send_sync_message(state: JSONObject, callbacks: any = {}): void {
+  send_sync_message(state: JSONObject, callbacks: any = {}): string {
     if (!this.comm) {
-      return;
+      return '';
     }
     try {
       callbacks.iopub = callbacks.iopub || {};
@@ -504,7 +555,7 @@ export class WidgetModel extends Backbone.Model {
 
       // split out the binary buffers
       const split = utils.remove_buffers(state);
-      this.comm.send(
+      const msgId = this.comm.send(
         {
           method: 'update',
           state: split.state,
@@ -515,9 +566,11 @@ export class WidgetModel extends Backbone.Model {
         split.buffers
       );
       this._pending_msgs++;
+      return msgId;
     } catch (e) {
       console.error('Could not send widget sync message', e);
     }
+    return '';
   }
 
   /**
@@ -624,6 +677,12 @@ export class WidgetModel extends Backbone.Model {
   private _msg_buffer: any;
   private _msg_buffer_callbacks: any;
   private _pending_msgs: number;
+  // keep track of the msg id for each attr for updates we send out so
+  // that we can ignore old messages that we send in order to avoid
+  // 'drunken' sliders going back and forward
+  private expectedEchoMsgIds: Map<string, string>;
+  // because we don't know the attrs in _handle_status, we keep track of what we will send
+  private attrsToUpdate: Set<string>;
 }
 
 export class DOMWidgetModel extends WidgetModel {

packages/schema/messages.md

@@ -292,6 +292,32 @@ The `data.state` and `data.buffer_paths` values are the same as in the `comm_ope
 
 See the [Model state](jupyterwidgetmodels.latest.md) documentation for the attributes of core Jupyter widgets.
 
+#### Synchronizing multiple frontends: `echo_update`
+
+Starting with protocol version `2.1.0`, `echo_update` messages from the kernel to the frontend are optional update messages for echoing state in messages from a frontend to the kernel back out to all the frontends.
+
+```
+{
+  'comm_id' : 'u-u-i-d',
+  'data' : {
+    'method': 'echo_update',
+    'state': { <dictionary of widget state> },
+    'buffer_paths': [ <list with paths corresponding to the binary buffers> ]
+  }
+}
+```
+
+The Jupyter comm protocol is asymmetric in how messages flow: messages flow from a single frontend to a single kernel, but messages are broadcast from the kernel to *all* frontends. In the widget protocol, if a frontend updates the value of a widget, the frontend does not have a way to directly notify other frontends about the state update. The `echo_update` optional messages enable a kernel to broadcast out frontend updates to all frontends. This can also help resolve the race condition where the kernel and a frontend simultaneously send updates to each other since the frontend now knows the order of kernel updates.
+
+The `echo_update` messages enable a frontend to optimistically update its widget views to reflect its own changes that it knows the kernel will yet process. These messages are intended to be used as follows:
+1. A frontend model attribute is updated, and the frontend views are optimistically updated to reflect the attribute.
+2. The frontend queues an update message to the kernel and records the message id for the attribute.
+3. The frontend ignores updates to the attribute from the kernel contained in `echo_update` messages until it gets an `echo_update` message corresponding to its own update of the attribute (i.e., the [parent_header](https://jupyter-client.readthedocs.io/en/latest/messaging.html#parent-header) id matches the stored message id for the attribute). It also ignores `echo_update` updates if it has a pending attribute update to send to the kernel. Once the frontend receives its own `echo_update` and does not have any more pending attribute updates to send to the kernel, it starts applying attribute updates from `echo_update` messages.
+
+Since the `echo_update` update messages are optional, and not all attribute updates may be echoed, it is important that only `echo_update` updates are ignored in the last step above, and `update` message updates are always applied.
+
+Implementation note: For attributes where sending back an `echo_update` is considered too expensive or unnecessary, we have implemented an opt-out mechanism in the ipywidgets package. A model trait can have the `echo_update` metadata attribute set to `False` to flag that the kernel should never send an `echo_update` update for that attribute to the frontends. Additionally, we have a system-wide flag to disable echoing for all attributes via the environment variable `JUPYTER_WIDGETS_ECHO`. For ipywdgets 7.7, we default `JUPYTER_WIDGETS_ECHO` to off (disabling all echo messages) and in ipywidgets 8.0 we default `JUPYTER_WIDGETS_ECHO` to on (enabling echo messages).
+
 #### State requests: `request_state`
 
 When a frontend wants to request the full state of a widget, the frontend sends a `request_state` message:

python/ipywidgets/ipywidgets/_version.py

@@ -3,7 +3,7 @@
 
 __version__ = '8.0.0b0'
 
-__protocol_version__ = '2.0.0'
+__protocol_version__ = '2.1.0'
 __control_protocol_version__ = '1.0.0'
 
 # These are *protocol* versions for each package, *not* npm versions. To check, look at each package's src/version.ts file for the protocol version the package implements.