fix: Errors on concurrent access with disabled cache

Kitware · Feb 27, 2024 · 6a51397 · 6a51397
1 parent b998863
commit 6a51397
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 27 deletions.
diff --git a/Sources/IO/Core/HttpDataSetReader/index.js b/Sources/IO/Core/HttpDataSetReader/index.js
@@ -205,8 +205,10 @@ function vtkHttpDataSetReader(publicAPI, model) {
       // cacheArrays[arrayId] can be a promise or value
       Promise.resolve(cachedArraysAndPromises[arrayId]).then((cachedArray) => {
         if (array !== cachedArray) {
-          //  Update last access for cache retention rules
-          cachedArraysMetaData[arrayId].lastAccess = new Date();
+          //  Update last access for cache retention rules (if caching is enabled)
+          if (model.maxCacheSize) {
+            cachedArraysMetaData[arrayId].lastAccess = new Date();
+          }
 
           // Assign cached array as result
           Object.assign(array, cachedArray);

diff --git a/Sources/IO/Core/HttpDataSetReader/test/MockDataAccessHelper.js b/Sources/IO/Core/HttpDataSetReader/test/MockDataAccessHelper.js
@@ -3,6 +3,7 @@ import macro from 'vtk.js/Sources/macros';
 
 const MockBaseURL = 'http://mockData';
 const MiB = 1024 * 1024;
+let fetchArrayDelayMs = 0;
 
 function createMockIndexJSON(fileId, byteLength) {
   return {
@@ -99,31 +100,33 @@ function fetchJSON(instance, url, options = {}) {
 function fetchArray(instance, baseURL, array, options = {}) {
   const url = `${baseURL}/${array.ref.basepath}/${array.ref.id}.gz`;
   const promise = new Promise((resolve, reject) => {
-    if (!baseURL.startsWith(MockBaseURL)) {
-      reject(new Error(`No such array ${url}`));
-      return;
-    }
-
-    const dataId = url.split('/').slice(-3)[0];
-    const filename = `${dataId}/${array.ref.basepath}/${array.ref.id}.gz`;
-
-    if (!MockData[filename]) {
-      reject(new Error(`No such array ${url}`));
-      return;
-    }
-
-    array.buffer = MockData[filename]();
-    array.values = macro.newTypedArray(array.dataType, array.buffer);
-    delete array.ref;
-
-    if (instance?.invokeBusy) {
-      instance.invokeBusy(false);
-    }
-    if (instance?.modified) {
-      instance.modified();
-    }
-    resolve(array);
-  });
+    setTimeout(() => {
+      if (!baseURL.startsWith(MockBaseURL)) {
+        reject(new Error(`No such array ${url}`));
+        return;
+      }
+
+      const dataId = url.split('/').slice(-3)[0];
+      const filename = `${dataId}/${array.ref.basepath}/${array.ref.id}.gz`;
+
+      if (!MockData[filename]) {
+        reject(new Error(`No such array ${url}`));
+        return;
+      }
+
+      array.buffer = MockData[filename]();
+      array.values = macro.newTypedArray(array.dataType, array.buffer);
+      delete array.ref;
+
+      if (instance?.invokeBusy) {
+        instance.invokeBusy(false);
+      }
+      if (instance?.modified) {
+        instance.modified();
+      }
+      resolve(array);
+    });
+  }, fetchArrayDelayMs);
 
   CallTrackers.forEach((t) => {
     t.fetchArray.push({
@@ -141,6 +144,10 @@ function fetchImage(instance, url, options = {}) {
   });
 }
 
+function setFetchArrayDelayMs(delay) {
+  fetchArrayDelayMs = delay;
+}
+
 // ----------------------------------------------------------------------------
 
 const MockDataAccessHelper = {
@@ -149,6 +156,7 @@ const MockDataAccessHelper = {
   fetchArray,
   fetchImage,
   getCallTracker,
+  setFetchArrayDelayMs,
 };
 
 registerType('mock', (options) => MockDataAccessHelper);

diff --git a/Sources/IO/Core/HttpDataSetReader/test/testHttpDataSetReader.js b/Sources/IO/Core/HttpDataSetReader/test/testHttpDataSetReader.js
@@ -237,3 +237,45 @@ test('Disabled cache on vtkHttpDataSetReader.', async (t) => {
 
   reader.setDataAccessHelper(originalAccessHelper);
 });
+
+test('Disabled cache does not raise error on concurrent access.', async (t) => {
+  const readers = [
+    vtkHttpDataSetReader.newInstance({ fetchGzip: true }),
+    vtkHttpDataSetReader.newInstance({ fetchGzip: true }),
+  ];
+
+  MockDataAccessHelper.setFetchArrayDelayMs(100);
+
+  readers.forEach((reader) => {
+    reader.setDataAccessHelper(MockDataAccessHelper);
+    reader.clearCache();
+  });
+  const disableOption = null;
+
+  await runTests([
+    // disable caching on all readers
+    (resolve, _) => {
+      readers.forEach((reader) => {
+        reader.setMaxCacheSize(disableOption);
+        t.equals(
+          reader.getMaxCacheSize(),
+          disableOption,
+          `Cache was disabled through setting maxCacheSize to "${disableOption}"`
+        );
+        resolve();
+      });
+    },
+
+    // ensure that concurrent calls from multiple readers does not raise errors
+    (resolve, _) => {
+      readers.forEach((reader) => {
+        const p = reader.setUrl('http://mockData/test01', { loadData: true });
+        p.then(() => {
+          resolve();
+        });
+      });
+    },
+  ]);
+
+  MockDataAccessHelper.setFetchArrayDelayMs(0);
+});