From 07397bbdde8bee94510c09373968c409ffe26ce4 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 20 Mar 2026 13:56:30 +0100 Subject: [PATCH] [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 --------- Co-authored-by: Eric Mc Sween GitOrigin-RevId: d3f171467d3bc26941758dd333f30049b37a05c8 --- services/clsi/app/js/CompileController.js | 18 ++--- services/clsi/app/js/CompileManager.js | 10 +++ services/clsi/app/js/LatexRunner.js | 8 +- services/clsi/app/js/LockManager.js | 19 ++++- .../clsi/test/acceptance/js/ClearCache.js | 73 +++++++++++++++++++ .../clsi/test/acceptance/js/StopCompile.js | 13 ++++ .../clsi/test/acceptance/js/helpers/Client.js | 2 +- 7 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 services/clsi/test/acceptance/js/ClearCache.js diff --git a/services/clsi/app/js/CompileController.js b/services/clsi/app/js/CompileController.js index 836efe2b58..ef865e9609 100644 --- a/services/clsi/app/js/CompileController.js +++ b/services/clsi/app/js/CompileController.js @@ -1,3 +1,4 @@ +import OError from '@overleaf/o-error' import Path from 'node:path' import RequestParser from './RequestParser.js' import CompileManager from './CompileManager.js' @@ -185,17 +186,14 @@ function stopCompile(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) - } - ) + }) + }) } function syncFromCode(req, res, next) { diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 104a214fc6..cf1e1b71e4 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -383,7 +383,17 @@ async function _readFdbFile(compileDir) { async function stopCompile(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 lockReleased } async function clearProject(projectId, userId) { diff --git a/services/clsi/app/js/LatexRunner.js b/services/clsi/app/js/LatexRunner.js index 0f875966eb..f0607ec670 100644 --- a/services/clsi/app/js/LatexRunner.js +++ b/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) { const id = `${projectId}` logger.debug({ id }, 'killing running compile') - if (ProcessTable[id] == null) { + if (!isRunning(projectId)) { logger.warn({ id }, 'no such project to kill') callback(null) } else { @@ -208,6 +213,7 @@ function _buildLatexCommand(mainFile, opts = {}) { } export default { + isRunning, runLatex, killLatex, promises: { diff --git a/services/clsi/app/js/LockManager.js b/services/clsi/app/js/LockManager.js index f7dc4bcc19..253884f72c 100644 --- a/services/clsi/app/js/LockManager.js +++ b/services/clsi/app/js/LockManager.js @@ -10,6 +10,14 @@ const LOCK_TIMEOUT_MS = RequestParser.MAX_TIMEOUT * 1000 + 120000 const LOCKS = new Map() +/** + * @param key + * @return {Lock | undefined} + */ +function getExistingLock(key) { + return LOCKS.get(key) +} + function acquire(key) { const currentLock = LOCKS.get(key) if (currentLock != null) { @@ -52,7 +60,16 @@ class Lock { return Date.now() >= this.expiresAt } + waitForRelease() { + if (this.waitingForRelease) return this.waitingForRelease + this.waitingForRelease = new Promise(resolve => { + this.onRelease = resolve + }) + return this.waitingForRelease + } + release() { + if (this.onRelease) this.onRelease() const lockWasActive = LOCKS.delete(this.key) if (!lockWasActive) { logger.error({ key: this.key }, 'Lock was released twice') @@ -63,4 +80,4 @@ class Lock { } } -export default { acquire } +export default { acquire, getExistingLock } diff --git a/services/clsi/test/acceptance/js/ClearCache.js b/services/clsi/test/acceptance/js/ClearCache.js new file mode 100644 index 0000000000..d9c4926e02 --- /dev/null +++ b/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) + }) +}) diff --git a/services/clsi/test/acceptance/js/StopCompile.js b/services/clsi/test/acceptance/js/StopCompile.js index f7b49e3150..54c7cbf3fe 100644 --- a/services/clsi/test/acceptance/js/StopCompile.js +++ b/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.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) + }) }) diff --git a/services/clsi/test/acceptance/js/helpers/Client.js b/services/clsi/test/acceptance/js/helpers/Client.js index 32b87ce38b..c397954a21 100644 --- a/services/clsi/test/acceptance/js/helpers/Client.js +++ b/services/clsi/test/acceptance/js/helpers/Client.js @@ -31,7 +31,7 @@ async function stopCompile(projectId) { } async function clearCache(projectId) { - await fetchNothing(`${host}/project/${projectId}`, { + return await fetchNothing(`${host}/project/${projectId}`, { method: 'DELETE', }) }