fix: prevent race condition in session cleanup timeout (#7471)

When the cleanup timeout fires, check the in-memory exp.real before
reading from the DB. If touch() extended the expiry (but the old
timeout fires late, e.g. on slow CI), reschedule instead of reading
potentially stale cached data from the DB and destroying the session.

Also increased test expiry times so the "touch after eligible for
refresh" test isn't sensitive to event loop delays on slow machines.

Fixes flaky SessionStore test from #7448.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/node/db/SessionStore.ts

@@ -122,15 +122,22 @@ class SessionStore extends expressSession.Store {
       // If reading from the database, update the expiration with the latest value from touch() so
       // that touch() appears to write to the database every time even though it doesn't.
       if (typeof expires === 'string') sess.cookie.expires = new Date(exp.real).toJSON();
-      // Use this._get(), not this._destroy(), to destroy the DB record for the expired session.
-      // This is done in case multiple Etherpad instances are sharing the same database and users
-      // are bouncing between the instances. By using this._get(), this instance will query the DB
-      // for the latest expiration time written by any of the instances, ensuring that the record
-      // isn't prematurely deleted if the expiration time was updated by a different Etherpad
-      // instance. (Important caveat: Client-side database caching, which ueberdb does by default,
-      // could still cause the record to be prematurely deleted because this instance might get a
-      // stale expiration time from cache.)
-      exp.timeout = setTimeout(() => this._get(sid), exp.real - now);
+      // Schedule cleanup when the session is expected to expire. When the timeout fires, check
+      // the in-memory expiry first — touch() may have extended it without rescheduling the timeout
+      // (e.g., if touch's clearTimeout raced with the timer on a slow system). If the session was
+      // extended, reschedule instead of reading from the DB which may return stale cached data.
+      exp.timeout = setTimeout(() => {
+        const currentExp = this._expirations.get(sid);
+        if (currentExp && currentExp.real > Date.now()) {
+          // Expiry was extended (e.g., by touch). Reschedule.
+          currentExp.timeout = setTimeout(() => this._get(sid), currentExp.real - Date.now());
+          return;
+        }
+        // Use this._get(), not this._destroy(), to query the DB for the latest expiration in case
+        // multiple Etherpad instances share the database. (Caveat: client-side DB caching could
+        // still cause premature deletion if the cache returns a stale expiration time.)
+        this._get(sid);
+      }, exp.real - now);
       this._expirations.set(sid, exp);
     } else {
       this._expirations.delete(sid);

src/tests/backend/specs/SessionStore.ts

@@ -224,10 +224,10 @@ describe(__filename, function () {
 
     it('touch after eligible for refresh updates db', async function () {
       const start = Date.now();
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(start + 200)}};
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(start + 2000)}};
       await set(sess);
       await new Promise((resolve) => setTimeout(resolve, 100));
-      const sess2:any  = {foo: 'bar', cookie: {expires: new Date(start + 400)}};
+      const sess2:any  = {foo: 'bar', cookie: {expires: new Date(start + 4000)}};
       await touch(sess2);
       await new Promise((resolve) => setTimeout(resolve, 110));
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess2));