handle the reset correctly by implementing canceller

Author: stephanwlee

tensorboard/components/tf_dashboard_common/data-loader-behavior.ts

@@ -106,6 +106,11 @@ namespace tf_dashboard_common {
         type: Object,
         value: () => new Map<CacheKey, LoadState>(),
       },
+
+      _canceller: {
+        type: Object,
+        value: () => new tf_backend.Canceller(),
+      },
     },
 
     observers: ['_dataToLoadChanged(isAttached, dataToLoad.*)'],
@@ -122,6 +127,8 @@ namespace tf_dashboard_common {
     reset() {
       // https://github.com/tensorflow/tensorboard/issues/1499
       // Cannot use the observer to observe `loadKey` changes directly.
+      this.cancelAsync(this._loadDataAsync);
+      if (this._canceller) this._canceller.cancelAll();
       if (this._dataLoadState) this._dataLoadState.clear();
       if (this.isAttached) this._loadData();
     },
@@ -138,6 +145,13 @@ namespace tf_dashboard_common {
     },
 
     detached() {
+      // Note: Cannot call canceller.cancelAll since it will poison the cache.
+      // detached gets called when a component gets unmounted from the document
+      // but it can be re-mounted. When remounted, poisoned cache will manifest.
+      // t=0: dataLoadState: 'a' = loading
+      // t=10: unmount
+      // t=20: request for 'a' resolves but we do not change the loadState
+      // because we do not want to set one if, instead, it was resetted at t=10.
       this.cancelAsync(this._loadDataAsync);
     },
 
@@ -149,48 +163,63 @@ namespace tf_dashboard_common {
 
     _loadDataImpl() {
       if (!this.active) return;
-
       this.cancelAsync(this._loadDataAsync);
-      this._loadDataAsync = this.async(() => {
-        // Read-only property have a special setter.
-        this._setDataLoading(true);
-
-        // Promises return cacheKeys of the data that were fetched.
-        const promises = this.dataToLoad
-          .filter((datum) => {
-            const cacheKey = this.getDataLoadName(datum);
-            return !this._dataLoadState.has(cacheKey);
-          })
-          .map((datum) => {
-            const cacheKey = this.getDataLoadName(datum);
-            this._dataLoadState.set(cacheKey, LoadState.LOADING);
-            return this.requestData(datum).then((value) => {
-              this._dataLoadState.set(cacheKey, LoadState.LOADED);
-              this.loadDataCallback(this, datum, value);
-              return cacheKey;
+      this._loadDataAsync = this.async(
+        this._canceller.cancellable((result) => {
+          if (result.cancelled) {
+            return;
+          }
+          // Read-only property have a special setter.
+          this._setDataLoading(true);
+
+          // Promises return cacheKeys of the data that were fetched.
+          const promises = this.dataToLoad
+            .filter((datum) => {
+              const cacheKey = this.getDataLoadName(datum);
+              return !this._dataLoadState.has(cacheKey);
+            })
+            .map((datum) => {
+              const cacheKey = this.getDataLoadName(datum);
+              this._dataLoadState.set(cacheKey, LoadState.LOADING);
+              return this.requestData(datum).then(
+                this._canceller.cancellable((result) => {
+                  // It was resetted. Do not notify of the response.
+                  if (!result.cancelled) {
+                    this._dataLoadState.set(cacheKey, LoadState.LOADED);
+                    this.loadDataCallback(this, datum, result.value);
+                  }
+                  return cacheKey;
+                })
+              );
             });
-          });
 
-        return Promise.all(promises).then((keysFetched) => {
-          const fetched = new Set(keysFetched);
-          const shouldNotify = this.dataToLoad.some((datum) =>
-            fetched.has(this.getDataLoadName(datum))
+          return Promise.all(promises).then(
+            this._canceller.cancellable((result) => {
+              // It was resetted. Do not notify of the data load.
+              if (!result.cancelled) {
+                const keysFetched = result.value;
+                const fetched = new Set(keysFetched);
+                const shouldNotify = this.dataToLoad.some((datum) =>
+                  fetched.has(this.getDataLoadName(datum))
+                );
+
+                if (shouldNotify) {
+                  this.onLoadFinish();
+                }
+              }
+
+              const isDataFetchPending = Array.from(
+                this._dataLoadState.values()
+              ).some((loadState) => loadState === LoadState.LOADING);
+
+              if (!isDataFetchPending) {
+                // Read-only property have a special setter.
+                this._setDataLoading(false);
+              }
+            })
           );
-
-          if (shouldNotify) {
-            this.onLoadFinish();
-          }
-
-          const isDataFetchPending = Array.from(
-            this._dataLoadState.values()
-          ).some((loadState) => loadState === LoadState.LOADING);
-
-          if (!isDataFetchPending) {
-            // Read-only property have a special setter.
-            this._setDataLoading(false);
-          }
-        });
-      });
+        })
+      );
     },
   };
 } // namespace tf_dashboard_common