Revert "[clsi] remove locking from docker actions (#32373)"

This reverts commit d66a856baa.

services/clsi/app/js/DockerLockManager.js

@@ -0,0 +1,110 @@
+// TODO: This file was created by bulk-decaffeinate.
+// Fix any style issues and re-enable lint.
+/*
+ * decaffeinate suggestions:
+ * DS101: Remove unnecessary use of Array.from
+ * DS102: Remove unnecessary code created because of implicit returns
+ * DS207: Consider shorter variations of null checks
+ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
+ */
+import logger from '@overleaf/logger'
+let LockManager
+
+const LockState = {} // locks for docker container operations, by container name
+
+export default LockManager = {
+  MAX_LOCK_HOLD_TIME: 15000, // how long we can keep a lock
+  MAX_LOCK_WAIT_TIME: 10000, // how long we wait for a lock
+  LOCK_TEST_INTERVAL: 1000, // retry time
+
+  tryLock(key, callback) {
+    let lockValue
+    if (callback == null) {
+      callback = function () {}
+    }
+    const existingLock = LockState[key]
+    if (existingLock != null) {
+      // the lock is already taken, check how old it is
+      const lockAge = Date.now() - existingLock.created
+      if (lockAge < LockManager.MAX_LOCK_HOLD_TIME) {
+        return callback(null, false) // we didn't get the lock, bail out
+      } else {
+        logger.error(
+          { key, lock: existingLock, age: lockAge },
+          'taking old lock by force'
+        )
+      }
+    }
+    // take the lock
+    LockState[key] = lockValue = { created: Date.now() }
+    return callback(null, true, lockValue)
+  },
+
+  getLock(key, callback) {
+    let attempt
+    if (callback == null) {
+      callback = function () {}
+    }
+    const startTime = Date.now()
+    return (attempt = () =>
+      LockManager.tryLock(key, function (error, gotLock, lockValue) {
+        if (error != null) {
+          return callback(error)
+        }
+        if (gotLock) {
+          return callback(null, lockValue)
+        } else if (Date.now() - startTime > LockManager.MAX_LOCK_WAIT_TIME) {
+          const e = new Error('Lock timeout')
+          e.key = key
+          return callback(e)
+        } else {
+          return setTimeout(attempt, LockManager.LOCK_TEST_INTERVAL)
+        }
+      }))()
+  },
+
+  releaseLock(key, lockValue, callback) {
+    if (callback == null) {
+      callback = function () {}
+    }
+    const existingLock = LockState[key]
+    if (existingLock === lockValue) {
+      // lockValue is an object, so we can test by reference
+      delete LockState[key] // our lock, so we can free it
+      return callback()
+    } else if (existingLock != null) {
+      // lock exists but doesn't match ours
+      logger.error(
+        { key, lock: existingLock },
+        'tried to release lock taken by force'
+      )
+      return callback()
+    } else {
+      logger.error(
+        { key, lock: existingLock },
+        'tried to release lock that has gone'
+      )
+      return callback()
+    }
+  },
+
+  runWithLock(key, runner, callback) {
+    if (callback == null) {
+      callback = function () {}
+    }
+    return LockManager.getLock(key, function (error, lockValue) {
+      if (error != null) {
+        return callback(error)
+      }
+      return runner((error1, ...args) =>
+        LockManager.releaseLock(key, lockValue, function (error2) {
+          error = error1 || error2
+          if (error != null) {
+            return callback(error)
+          }
+          return callback(null, ...Array.from(args))
+        })
+      )
+    })
+  },
+}

services/clsi/test/unit/js/DockerLockManager.test.js

@@ -0,0 +1,259 @@
+import { vi, describe, beforeEach, it } from 'vitest'
+
+import sinon from 'sinon'
+import path from 'node:path'
+const modulePath = path.join(
+  import.meta.dirname,
+  '../../../app/js/DockerLockManager'
+)
+
+describe('DockerLockManager', function () {
+  beforeEach(async function (ctx) {
+    vi.doMock('@overleaf/settings', () => ({
+      default: (ctx.Settings = { clsi: { docker: {} } }),
+    }))
+
+    return (ctx.LockManager = (await import(modulePath)).default)
+  })
+
+  return describe('runWithLock', function () {
+    describe('with a single lock', function () {
+      beforeEach(async function (ctx) {
+        await new Promise((resolve, reject) => {
+          ctx.callback = sinon.stub()
+          return ctx.LockManager.runWithLock(
+            'lock-one',
+            releaseLock =>
+              setTimeout(() => releaseLock(null, 'hello', 'world'), 100),
+
+            (err, ...args) => {
+              ctx.callback(err, ...Array.from(args))
+              return resolve()
+            }
+          )
+        })
+      })
+
+      return it('should call the callback', function (ctx) {
+        return ctx.callback
+          .calledWith(null, 'hello', 'world')
+          .should.equal(true)
+      })
+    })
+
+    describe('with two locks', function () {
+      beforeEach(async function (ctx) {
+        await new Promise((resolve, reject) => {
+          ctx.callback1 = sinon.stub()
+          ctx.callback2 = sinon.stub()
+          ctx.LockManager.runWithLock(
+            'lock-one',
+            releaseLock =>
+              setTimeout(() => releaseLock(null, 'hello', 'world', 'one'), 100),
+
+            (err, ...args) => {
+              return ctx.callback1(err, ...Array.from(args))
+            }
+          )
+          return ctx.LockManager.runWithLock(
+            'lock-two',
+            releaseLock =>
+              setTimeout(() => releaseLock(null, 'hello', 'world', 'two'), 200),
+
+            (err, ...args) => {
+              ctx.callback2(err, ...Array.from(args))
+              return resolve()
+            }
+          )
+        })
+      })
+
+      it('should call the first callback', function (ctx) {
+        return ctx.callback1
+          .calledWith(null, 'hello', 'world', 'one')
+          .should.equal(true)
+      })
+
+      return it('should call the second callback', function (ctx) {
+        return ctx.callback2
+          .calledWith(null, 'hello', 'world', 'two')
+          .should.equal(true)
+      })
+    })
+
+    return describe('with lock contention', function () {
+      describe('where the first lock is released quickly', function () {
+        beforeEach(async function (ctx) {
+          await new Promise((resolve, reject) => {
+            ctx.LockManager.MAX_LOCK_WAIT_TIME = 1000
+            ctx.LockManager.LOCK_TEST_INTERVAL = 100
+            ctx.callback1 = sinon.stub()
+            ctx.callback2 = sinon.stub()
+            ctx.LockManager.runWithLock(
+              'lock',
+              releaseLock =>
+                setTimeout(
+                  () => releaseLock(null, 'hello', 'world', 'one'),
+                  100
+                ),
+
+              (err, ...args) => {
+                return ctx.callback1(err, ...Array.from(args))
+              }
+            )
+            return ctx.LockManager.runWithLock(
+              'lock',
+              releaseLock =>
+                setTimeout(
+                  () => releaseLock(null, 'hello', 'world', 'two'),
+                  200
+                ),
+
+              (err, ...args) => {
+                ctx.callback2(err, ...Array.from(args))
+                return resolve()
+              }
+            )
+          })
+        })
+
+        it('should call the first callback', function (ctx) {
+          return ctx.callback1
+            .calledWith(null, 'hello', 'world', 'one')
+            .should.equal(true)
+        })
+
+        return it('should call the second callback', function (ctx) {
+          return ctx.callback2
+            .calledWith(null, 'hello', 'world', 'two')
+            .should.equal(true)
+        })
+      })
+
+      describe('where the first lock is held longer than the waiting time', function () {
+        beforeEach(async function (ctx) {
+          await new Promise((resolve, reject) => {
+            let doneTwo
+            ctx.LockManager.MAX_LOCK_HOLD_TIME = 10000
+            ctx.LockManager.MAX_LOCK_WAIT_TIME = 1000
+            ctx.LockManager.LOCK_TEST_INTERVAL = 100
+            ctx.callback1 = sinon.stub()
+            ctx.callback2 = sinon.stub()
+            let doneOne = (doneTwo = false)
+            const finish = function (key) {
+              if (key === 1) {
+                doneOne = true
+              }
+              if (key === 2) {
+                doneTwo = true
+              }
+              if (doneOne && doneTwo) {
+                return resolve()
+              }
+            }
+            ctx.LockManager.runWithLock(
+              'lock',
+              releaseLock =>
+                setTimeout(
+                  () => releaseLock(null, 'hello', 'world', 'one'),
+                  1100
+                ),
+
+              (err, ...args) => {
+                ctx.callback1(err, ...Array.from(args))
+                return finish(1)
+              }
+            )
+            return ctx.LockManager.runWithLock(
+              'lock',
+              releaseLock =>
+                setTimeout(
+                  () => releaseLock(null, 'hello', 'world', 'two'),
+                  100
+                ),
+
+              (err, ...args) => {
+                ctx.callback2(err, ...Array.from(args))
+                return finish(2)
+              }
+            )
+          })
+        })
+
+        it('should call the first callback', function (ctx) {
+          return ctx.callback1
+            .calledWith(null, 'hello', 'world', 'one')
+            .should.equal(true)
+        })
+
+        return it('should call the second callback with an error', function (ctx) {
+          const error = sinon.match.instanceOf(Error)
+          return ctx.callback2.calledWith(error).should.equal(true)
+        })
+      })
+
+      return describe('where the first lock is held longer than the max holding time', function () {
+        beforeEach(async function (ctx) {
+          await new Promise((resolve, reject) => {
+            let doneTwo
+            ctx.LockManager.MAX_LOCK_HOLD_TIME = 1000
+            ctx.LockManager.MAX_LOCK_WAIT_TIME = 2000
+            ctx.LockManager.LOCK_TEST_INTERVAL = 100
+            ctx.callback1 = sinon.stub()
+            ctx.callback2 = sinon.stub()
+            let doneOne = (doneTwo = false)
+            const finish = function (key) {
+              if (key === 1) {
+                doneOne = true
+              }
+              if (key === 2) {
+                doneTwo = true
+              }
+              if (doneOne && doneTwo) {
+                return resolve()
+              }
+            }
+            ctx.LockManager.runWithLock(
+              'lock',
+              releaseLock =>
+                setTimeout(
+                  () => releaseLock(null, 'hello', 'world', 'one'),
+                  1500
+                ),
+
+              (err, ...args) => {
+                ctx.callback1(err, ...Array.from(args))
+                return finish(1)
+              }
+            )
+            return ctx.LockManager.runWithLock(
+              'lock',
+              releaseLock =>
+                setTimeout(
+                  () => releaseLock(null, 'hello', 'world', 'two'),
+                  100
+                ),
+
+              (err, ...args) => {
+                ctx.callback2(err, ...Array.from(args))
+                return finish(2)
+              }
+            )
+          })
+        })
+
+        it('should call the first callback', function (ctx) {
+          return ctx.callback1
+            .calledWith(null, 'hello', 'world', 'one')
+            .should.equal(true)
+        })
+
+        return it('should call the second callback', function (ctx) {
+          return ctx.callback2
+            .calledWith(null, 'hello', 'world', 'two')
+            .should.equal(true)
+        })
+      })
+    })
+  })
+})