Răsfoiți Sursa

[clsi] avoid server error when clearing cache while compiling (#32349)

* [clsi] avoid server error when clearing cache while compiling

* [clsi] tweak API around releasing locks

Co-authored-by: Eric Mc Sween <eric.mcsween@overleaf.com>

---------

Co-authored-by: Eric Mc Sween <eric.mcsween@overleaf.com>
GitOrigin-RevId: d3f171467d3bc26941758dd333f30049b37a05c8
Jakob Ackermann 6 luni în urmă
părinte
comite
07397bbdde

+ 8 - 10
services/clsi/app/js/CompileController.js

@@ -1,3 +1,4 @@
+import OError from '@overleaf/o-error'
 import Path from 'node:path'
 import Path from 'node:path'
 import RequestParser from './RequestParser.js'
 import RequestParser from './RequestParser.js'
 import CompileManager from './CompileManager.js'
 import CompileManager from './CompileManager.js'
@@ -185,17 +186,14 @@ function stopCompile(req, res, next) {
 }
 }
 
 
 function clearCache(req, res, next) {
 function clearCache(req, res, next) {
-  ProjectPersistenceManager.clearProject(
-    req.params.project_id,
-    req.params.user_id,
-    function (error) {
-      if (error) {
-        return next(error)
-      }
-      // No content
+  const { project_id: projectId, user_id: userId } = req.params
+  CompileManager.stopCompile(projectId, userId, error => {
+    if (error) return next(OError.tag(error, 'stop compile'))
+    ProjectPersistenceManager.clearProject(projectId, userId, error => {
+      if (error) return next(OError.tag(error, 'clear project'))
       res.sendStatus(204)
       res.sendStatus(204)
-    }
-  )
+    })
+  })
 }
 }
 
 
 function syncFromCode(req, res, next) {
 function syncFromCode(req, res, next) {

+ 10 - 0
services/clsi/app/js/CompileManager.js

@@ -383,7 +383,17 @@ async function _readFdbFile(compileDir) {
 
 
 async function stopCompile(projectId, userId) {
 async function stopCompile(projectId, userId) {
   const compileName = getCompileName(projectId, userId)
   const compileName = getCompileName(projectId, userId)
+  const lock = LockManager.getExistingLock(getCompileDir(projectId, userId))
+  let lockReleased
+  if (lock) {
+    lockReleased = lock.waitForRelease()
+  } else {
+    if (!LatexRunner.isRunning(compileName)) return
+    logger.warn({ projectId, userId }, 'found running compile without lock')
+    lockReleased = Promise.resolve()
+  }
   await LatexRunner.promises.killLatex(compileName)
   await LatexRunner.promises.killLatex(compileName)
+  await lockReleased
 }
 }
 
 
 async function clearProject(projectId, userId) {
 async function clearProject(projectId, userId) {

+ 7 - 1
services/clsi/app/js/LatexRunner.js

@@ -144,10 +144,15 @@ function _writeLogOutput(projectId, directory, output, callback) {
   })
   })
 }
 }
 
 
+function isRunning(projectId) {
+  const id = `${projectId}`
+  return ProcessTable[id] != null
+}
+
 function killLatex(projectId, callback) {
 function killLatex(projectId, callback) {
   const id = `${projectId}`
   const id = `${projectId}`
   logger.debug({ id }, 'killing running compile')
   logger.debug({ id }, 'killing running compile')
-  if (ProcessTable[id] == null) {
+  if (!isRunning(projectId)) {
     logger.warn({ id }, 'no such project to kill')
     logger.warn({ id }, 'no such project to kill')
     callback(null)
     callback(null)
   } else {
   } else {
@@ -208,6 +213,7 @@ function _buildLatexCommand(mainFile, opts = {}) {
 }
 }
 
 
 export default {
 export default {
+  isRunning,
   runLatex,
   runLatex,
   killLatex,
   killLatex,
   promises: {
   promises: {

+ 18 - 1
services/clsi/app/js/LockManager.js

@@ -10,6 +10,14 @@ const LOCK_TIMEOUT_MS = RequestParser.MAX_TIMEOUT * 1000 + 120000
 
 
 const LOCKS = new Map()
 const LOCKS = new Map()
 
 
+/**
+ * @param key
+ * @return {Lock | undefined}
+ */
+function getExistingLock(key) {
+  return LOCKS.get(key)
+}
+
 function acquire(key) {
 function acquire(key) {
   const currentLock = LOCKS.get(key)
   const currentLock = LOCKS.get(key)
   if (currentLock != null) {
   if (currentLock != null) {
@@ -52,7 +60,16 @@ class Lock {
     return Date.now() >= this.expiresAt
     return Date.now() >= this.expiresAt
   }
   }
 
 
+  waitForRelease() {
+    if (this.waitingForRelease) return this.waitingForRelease
+    this.waitingForRelease = new Promise(resolve => {
+      this.onRelease = resolve
+    })
+    return this.waitingForRelease
+  }
+
   release() {
   release() {
+    if (this.onRelease) this.onRelease()
     const lockWasActive = LOCKS.delete(this.key)
     const lockWasActive = LOCKS.delete(this.key)
     if (!lockWasActive) {
     if (!lockWasActive) {
       logger.error({ key: this.key }, 'Lock was released twice')
       logger.error({ key: this.key }, 'Lock was released twice')
@@ -63,4 +80,4 @@ class Lock {
   }
   }
 }
 }
 
 
-export default { acquire }
+export default { acquire, getExistingLock }

+ 73 - 0
services/clsi/test/acceptance/js/ClearCache.js

@@ -0,0 +1,73 @@
+import { promisify } from 'node:util'
+import Client from './helpers/Client.js'
+import ClsiApp from './helpers/ClsiApp.js'
+import { expect } from 'chai'
+
+const sleep = promisify(setTimeout)
+
+describe('Clear cache', function () {
+  before(async function () {
+    this.request = {
+      options: {
+        timeout: 100,
+      }, // seconds
+      resources: [
+        {
+          path: 'main.tex',
+          content: `\
+\\documentclass{article}
+\\begin{document}
+\\def\\x{Hello!\\par\\x}
+\\x
+\\end{document}\
+`,
+        },
+      ],
+    }
+    this.project_id = Client.randomId()
+    await ClsiApp.ensureRunning()
+
+    // start the compile in the background
+    Client.compile(this.project_id, this.request)
+      .then(body => {
+        this.compileResult = { body }
+      })
+      .catch(error => {
+        this.compileResult = { error }
+      })
+
+    // wait for 1 second before stopping the compile
+    await sleep(1000)
+
+    try {
+      const res = await Client.clearCache(this.project_id)
+      this.stopResult = { res }
+    } catch (error) {
+      this.stopResult = { error }
+    }
+
+    // allow time for the compile request to terminate
+    await sleep(1000)
+  })
+
+  it('should emit a compile response with terminated status', function () {
+    expect(this.stopResult.error).not.to.exist
+    expect(this.stopResult.res.status).to.equal(204)
+    expect(this.compileResult.error).not.to.exist
+    expect(this.compileResult.body.compile.status).to.equal('terminated')
+    expect(this.compileResult.body.compile.error).to.equal('terminated')
+  })
+
+  it('should return the log output file name', function () {
+    const outputFilePaths = this.compileResult.body.compile.outputFiles.map(
+      x => x.path
+    )
+    outputFilePaths.should.include('output.synctex(busy)') // compile was still pending
+    outputFilePaths.should.include('output.log')
+  })
+
+  it('should work with not pending compile', async function () {
+    const res = await Client.clearCache(this.project_id)
+    expect(res.status).to.equal(204)
+  })
+})

+ 13 - 0
services/clsi/test/acceptance/js/StopCompile.js

@@ -57,4 +57,17 @@ describe('Stop compile', function () {
     expect(this.compileResult.body.compile.status).to.equal('terminated')
     expect(this.compileResult.body.compile.status).to.equal('terminated')
     expect(this.compileResult.body.compile.error).to.equal('terminated')
     expect(this.compileResult.body.compile.error).to.equal('terminated')
   })
   })
+
+  it('should return the log output file name', function () {
+    const outputFilePaths = this.compileResult.body.compile.outputFiles.map(
+      x => x.path
+    )
+    outputFilePaths.should.include('output.synctex(busy)') // compile was still pending
+    outputFilePaths.should.include('output.log')
+  })
+
+  it('should work with not pending compile', async function () {
+    const res = await Client.stopCompile(this.project_id)
+    expect(res.status).to.equal(204)
+  })
 })
 })

+ 1 - 1
services/clsi/test/acceptance/js/helpers/Client.js

@@ -31,7 +31,7 @@ async function stopCompile(projectId) {
 }
 }
 
 
 async function clearCache(projectId) {
 async function clearCache(projectId) {
-  await fetchNothing(`${host}/project/${projectId}`, {
+  return await fetchNothing(`${host}/project/${projectId}`, {
     method: 'DELETE',
     method: 'DELETE',
   })
   })
 }
 }