fix: do not accept new update requests when state is detaching

src/client/Context.js

@@ -9,6 +9,9 @@ import {
   CONTEXT_EXIT_ERROR,
 } from '../common/constants.js';
 
+// share between all context, as channels are common to all contexts
+const promiseStore = new PromiseStore('Context');
+
 /**
  * Base class to extend in order to implement a new Context.
  *
@@ -77,39 +80,36 @@ class Context {
      */
     this.status = 'idle';
 
-    /** @private */
-    this._promiseStore = new PromiseStore(this.constructor.name);
-
     this.client.socket.addListener(CONTEXT_ENTER_RESPONSE, (reqId, contextName) => {
       if (contextName !== this.name) {
         return;
       }
 
-      this._promiseStore.resolve(reqId);
+      promiseStore.resolve(reqId);
     });
 
     this.client.socket.addListener(CONTEXT_ENTER_ERROR, (reqId, contextName, msg) => {
       if (contextName !== this.name) {
         return;
       }
 
-      this._promiseStore.reject(reqId, msg);
+      promiseStore.reject(reqId, msg);
     });
 
     this.client.socket.addListener(CONTEXT_EXIT_RESPONSE, (reqId, contextName) => {
       if (contextName !== this.name) {
         return;
       }
 
-      this._promiseStore.resolve(reqId);
+      promiseStore.resolve(reqId);
     });
 
     this.client.socket.addListener(CONTEXT_EXIT_ERROR, (reqId, contextName, msg) => {
       if (contextName !== this.name) {
         return;
       }
 
-      this._promiseStore.reject(reqId, msg);
+      promiseStore.reject(reqId, msg);
     });
 
     this.client.contextManager.register(this);
@@ -205,7 +205,7 @@ class Context {
     // we need the try/catch block to change the promise rejection into proper error
     try {
       await new Promise((resolve, reject) => {
-        const reqId = this._promiseStore.add(resolve, reject, 'enter-context');
+        const reqId = promiseStore.add(resolve, reject, 'enter-context');
         this.client.socket.send(CONTEXT_ENTER_REQUEST, reqId, this.name);
       });
     } catch (err) {
@@ -239,7 +239,7 @@ class Context {
     // we need the try/catch block to change the promise rejection into proper error
     try {
       await new Promise((resolve, reject) => {
-        const reqId = this._promiseStore.add(resolve, reject, 'exit-context');
+        const reqId = promiseStore.add(resolve, reject, 'exit-context');
         this.client.socket.send(CONTEXT_EXIT_REQUEST, reqId, this.name);
       });
     } catch (err) {

src/common/BaseSharedState.js

@@ -320,6 +320,10 @@ ${JSON.stringify(initValues, null, 2)}`);
    * const updates = await state.set({ myParam: Math.random() });
    */
   async set(updates, context = null) {
+    if (this._detached) {
+      return;
+    }
+
     if (!isPlainObject(updates)) {
       throw new ReferenceError(`[SharedState] State "${this.schemaName}": state.set(updates[, context]) should receive an object as first parameter`);
     }

src/common/BaseSharedStateCollection.js

@@ -3,9 +3,7 @@ import {
   kAttachInCollection,
 } from './BaseStateManager.js';
 
-/**
- * @private
- */
+/** @private */
 class BaseSharedStateCollection {
   constructor(stateManager, schemaName, options = {}) {
     this._stateManager = stateManager;
@@ -21,6 +19,7 @@ class BaseSharedStateCollection {
     this._unobserve = null;
   }
 
+  /** @private */
   async _init() {
     this._controller = await this._stateManager[kCreateCollectionController](this._schemaName);
     this._controller.onUpdate(async (updates, context) => {

src/common/PromiseStore.js

@@ -1,32 +1,39 @@
 import { idGenerator } from '@ircam/sc-utils';
 
-const generateRequestId = idGenerator();
-
 export default class PromiseStore {
   constructor(name) {
     this.name = name;
     this.store = new Map();
+    this.generateRequestId = idGenerator();
   }
 
   add(resolve, reject, type) {
-    const reqId = generateRequestId.next().value;
+    const reqId = this.generateRequestId.next().value;
     this.store.set(reqId, { resolve, reject, type });
 
     return reqId;
   }
 
   resolve(reqId, data) {
-    const { resolve } = this.store.get(reqId);
-    this.store.delete(reqId);
+    if (this.store.has(reqId)) {
+      const { resolve } = this.store.get(reqId);
+      this.store.delete(reqId);
 
-    resolve(data);
+      resolve(data);
+    } else {
+      throw new Error(`[${this.name}] cannot resolve request id (${reqId}), id does not exist`);
+    }
   }
 
   reject(reqId, msg) {
-    const { reject } = this.store.get(reqId);
-    this.store.delete(reqId);
+    if (this.store.has(reqId)) {
+      const { reject } = this.store.get(reqId);
+      this.store.delete(reqId);
 
-    reject(new Error(msg));
+      reject(new Error(msg));
+    } else {
+      throw new Error(`[${this.name}] cannot reject request id (${reqId}), id does not exist`);
+    }
   }
 
   // reject all pendeing request

tests/misc/PromiseStore.spec.js

@@ -0,0 +1,142 @@
+import { assert } from 'chai';
+import PromiseStore from '../../src/common/PromiseStore.js';
+
+import { Server } from '../../src/server/index.js';
+import { Client } from '../../src/client/index.js';
+
+import config from '../utils/config.js';
+import { a } from '../utils/schemas.js';
+
+describe('# PromiseStore', () => {
+  describe('## resolve(reqId)', () => {
+    it('should fail with meaningfull error if reqId does not exists', () => {
+      const store = new PromiseStore('test');
+
+      let errored = false;
+
+      try {
+        store.resolve(null);
+      } catch(err) {
+        console.log(err.message);
+        errored = true;
+      }
+
+      if (!errored) {
+        assert.fail('should have thrown');
+      }
+    });
+  });
+
+  describe('## reject(reqId)', () => {
+    it('should fail with meaningfull error if reqId does not exists', () => {
+      const store = new PromiseStore('test');
+
+      let errored = false;
+
+      try {
+        store.reject(null);
+      } catch(err) {
+        console.log(err.message);
+        errored = true;
+      }
+
+      if (!errored) {
+        assert.fail('should have thrown');
+      }
+    });
+  });
+
+
+  describe(`## MISC`, () => {
+    it(`check consistency of PromiseStore in SharedState`, async () => {
+      const server = new Server(config);
+      server.stateManager.registerSchema('a', a);
+      await server.start();
+
+      const client = new Client({ role: 'test', ...config });
+      await client.start();
+
+      const state = await client.stateManager.create('a');
+      const attached = await client.stateManager.attach('a');
+
+      { // update promise
+        const promise = state.set({ bool: true });
+        assert.equal(state._promiseStore.store.size, 1);
+        assert.equal(attached._promiseStore.store.size, 0);
+
+        await promise;
+
+        assert.equal(state._promiseStore.store.size, 0);
+        assert.equal(attached._promiseStore.store.size, 0);
+      }
+
+      { // update promise
+        const promise = attached.set({ int: 42 });
+        assert.equal(state._promiseStore.store.size, 0);
+        assert.equal(attached._promiseStore.store.size, 1);
+
+        await promise;
+
+        assert.equal(state._promiseStore.store.size, 0);
+        assert.equal(attached._promiseStore.store.size, 0);
+      }
+
+      { // update fail promise, fail early, no promise created
+        try {
+          await state.set({ int: null });
+        } catch(err) {
+          console.log(err.message);
+        }
+
+        assert.equal(state._promiseStore.store.size, 0);
+        assert.equal(attached._promiseStore.store.size, 0);
+      }
+
+      { // detach request
+        const promise = attached.detach();
+        assert.equal(state._promiseStore.store.size, 0);
+        assert.equal(attached._promiseStore.store.size, 1);
+
+        await promise;
+
+        assert.equal(state._promiseStore.store.size, 0);
+        assert.equal(attached._promiseStore.store.size, 0);
+      }
+
+      { // detach request
+        const promise = state.delete();
+        assert.equal(state._promiseStore.store.size, 1);
+
+        await promise;
+
+        assert.equal(state._promiseStore.store.size, 0);
+      }
+
+      await server.stop();
+    });
+
+    it(`stress test`, async () => {
+      const server = new Server(config);
+      server.stateManager.registerSchema('a', a);
+      await server.start();
+
+      const client = new Client({ role: 'test', ...config });
+      await client.start();
+
+      const state = await client.stateManager.create('a');
+
+      const promises = [];
+      for (let i = 0; i < 1e4; i++) {
+        let promise = state.set({ int: Math.floor(Math.random() * 1e12) });
+        assert.equal(state._promiseStore.store.size, i + 1);
+        promises.push(promise);
+      }
+
+      assert.equal(state._promiseStore.store.size, 1e4);
+      await Promise.all(promises);
+      assert.equal(state._promiseStore.store.size, 0);
+
+      await server.stop();
+    });
+  });
+});