From 7f0e6f3eec781e776fc3305d05d279d2dcad9b53 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 22 Sep 2017 16:19:33 +0100 Subject: [PATCH] lock compile directory --- .../clsi/app/coffee/CompileController.coffee | 7 ++- .../clsi/app/coffee/CompileManager.coffee | 13 +++++ services/clsi/app/coffee/Errors.coffee | 8 +++ services/clsi/app/coffee/LockManager.coffee | 23 ++++++++ .../clsi/app/coffee/ResourceWriter.coffee | 2 +- services/clsi/package.json | 1 + .../unit/coffee/CompileControllerTests.coffee | 10 ++-- .../unit/coffee/CompileManagerTests.coffee | 41 ++++++++++++++ .../clsi/test/unit/coffee/LockManager.coffee | 54 +++++++++++++++++++ 9 files changed, 151 insertions(+), 8 deletions(-) create mode 100644 services/clsi/app/coffee/LockManager.coffee create mode 100644 services/clsi/test/unit/coffee/LockManager.coffee diff --git a/services/clsi/app/coffee/CompileController.coffee b/services/clsi/app/coffee/CompileController.coffee index f4dcb0ec66..60c5c64c0b 100644 --- a/services/clsi/app/coffee/CompileController.coffee +++ b/services/clsi/app/coffee/CompileController.coffee @@ -15,8 +15,11 @@ module.exports = CompileController = request.user_id = req.params.user_id if req.params.user_id? ProjectPersistenceManager.markProjectAsJustAccessed request.project_id, (error) -> return next(error) if error? - CompileManager.doCompile request, (error, outputFiles = []) -> - if error instanceof Errors.FilesOutOfSyncError + CompileManager.doCompileWithLock request, (error, outputFiles = []) -> + if error instanceof Errors.AlreadyCompilingError + code = 423 # Http 443 Locked + status = "compile-in-progress" + else if error instanceof Errors.FilesOutOfSyncError code = 409 # Http 409 Conflict status = "retry" else if error?.terminated diff --git a/services/clsi/app/coffee/CompileManager.coffee b/services/clsi/app/coffee/CompileManager.coffee index 56d04db737..c4f3c7bfda 100644 --- a/services/clsi/app/coffee/CompileManager.coffee +++ b/services/clsi/app/coffee/CompileManager.coffee @@ -9,6 +9,7 @@ Metrics = require "./Metrics" child_process = require "child_process" DraftModeManager = require "./DraftModeManager" TikzManager = require "./TikzManager" +LockManager = require "./LockManager" fs = require("fs") fse = require "fs-extra" os = require("os") @@ -26,6 +27,18 @@ getCompileDir = (project_id, user_id) -> Path.join(Settings.path.compilesDir, getCompileName(project_id, user_id)) module.exports = CompileManager = + + doCompileWithLock: (request, callback = (error, outputFiles) ->) -> + compileDir = getCompileDir(request.project_id, request.user_id) + lockFile = Path.join(compileDir, ".project-lock") + # use a .project-lock file in the compile directory to prevent + # simultaneous compiles + fse.ensureDir compileDir, (error) -> + return callback(error) if error? + LockManager.runWithLock lockFile, (releaseLock) -> + CompileManager.doCompile(request, releaseLock) + , callback + doCompile: (request, callback = (error, outputFiles) ->) -> compileDir = getCompileDir(request.project_id, request.user_id) diff --git a/services/clsi/app/coffee/Errors.coffee b/services/clsi/app/coffee/Errors.coffee index 2e3ae7597d..b375513e1c 100644 --- a/services/clsi/app/coffee/Errors.coffee +++ b/services/clsi/app/coffee/Errors.coffee @@ -12,6 +12,14 @@ FilesOutOfSyncError = (message) -> return error FilesOutOfSyncError.prototype.__proto__ = Error.prototype +AlreadyCompilingError = (message) -> + error = new Error(message) + error.name = "AlreadyCompilingError" + error.__proto__ = AlreadyCompilingError.prototype + return error +AlreadyCompilingError.prototype.__proto__ = Error.prototype + module.exports = Errors = NotFoundError: NotFoundError FilesOutOfSyncError: FilesOutOfSyncError + AlreadyCompilingError: AlreadyCompilingError diff --git a/services/clsi/app/coffee/LockManager.coffee b/services/clsi/app/coffee/LockManager.coffee new file mode 100644 index 0000000000..5d6fa4626d --- /dev/null +++ b/services/clsi/app/coffee/LockManager.coffee @@ -0,0 +1,23 @@ +Settings = require('settings-sharelatex') +logger = require "logger-sharelatex" +Lockfile = require('lockfile') # from https://github.com/npm/lockfile +Errors = require "./Errors" + +module.exports = LockManager = + LOCK_TEST_INTERVAL: 1000 # 50ms between each test of the lock + MAX_LOCK_WAIT_TIME: 15000 # 10s maximum time to spend trying to get the lock + LOCK_STALE: 5*60*1000 # 5 mins time until lock auto expires + + runWithLock: (path, runner = ((releaseLock = (error) ->) ->), callback = ((error) ->)) -> + lockOpts = + wait: @MAX_LOCK_WAIT_TIME + pollPeriod: @LOCK_TEST_INTERVAL + stale: @LOCK_STALE + Lockfile.lock path, lockOpts, (error) -> + return callback new Errors.AlreadyCompilingError("compile in progress") if error?.code is 'EEXIST' + return callback(error) if error? + runner (error1, args...) -> + Lockfile.unlock path, (error2) -> + error = error1 or error2 + return callback(error) if error? + callback(null, args...) diff --git a/services/clsi/app/coffee/ResourceWriter.coffee b/services/clsi/app/coffee/ResourceWriter.coffee index f9e90b036a..06d5692f0a 100644 --- a/services/clsi/app/coffee/ResourceWriter.coffee +++ b/services/clsi/app/coffee/ResourceWriter.coffee @@ -78,7 +78,7 @@ module.exports = ResourceWriter = should_delete = true if path.match(/^output\./) or path.match(/\.aux$/) or path.match(/^cache\//) # knitr cache should_delete = false - if path == '.project-sync-state' + if path == '.project-sync-state' or path == '.project-lock' should_delete = false if path == "output.pdf" or path == "output.dvi" or path == "output.log" or path == "output.xdv" should_delete = true diff --git a/services/clsi/package.json b/services/clsi/package.json index 56843026e7..71711be1a7 100644 --- a/services/clsi/package.json +++ b/services/clsi/package.json @@ -14,6 +14,7 @@ "fs-extra": "^0.16.3", "grunt-mkdir": "^1.0.0", "heapdump": "^0.3.5", + "lockfile": "^1.0.3", "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#v1.5.4", "lynx": "0.0.11", "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.5.0", diff --git a/services/clsi/test/unit/coffee/CompileControllerTests.coffee b/services/clsi/test/unit/coffee/CompileControllerTests.coffee index 1fc6a99bd5..7b6001d0fa 100644 --- a/services/clsi/test/unit/coffee/CompileControllerTests.coffee +++ b/services/clsi/test/unit/coffee/CompileControllerTests.coffee @@ -49,7 +49,7 @@ describe "CompileController", -> describe "successfully", -> beforeEach -> - @CompileManager.doCompile = sinon.stub().callsArgWith(1, null, @output_files) + @CompileManager.doCompileWithLock = sinon.stub().callsArgWith(1, null, @output_files) @CompileController.compile @req, @res it "should parse the request", -> @@ -58,7 +58,7 @@ describe "CompileController", -> .should.equal true it "should run the compile for the specified project", -> - @CompileManager.doCompile + @CompileManager.doCompileWithLock .calledWith(@request_with_project_id) .should.equal true @@ -84,7 +84,7 @@ describe "CompileController", -> describe "with an error", -> beforeEach -> - @CompileManager.doCompile = sinon.stub().callsArgWith(1, new Error(@message = "error message"), null) + @CompileManager.doCompileWithLock = sinon.stub().callsArgWith(1, new Error(@message = "error message"), null) @CompileController.compile @req, @res it "should return the JSON response with the error", -> @@ -102,7 +102,7 @@ describe "CompileController", -> beforeEach -> @error = new Error(@message = "container timed out") @error.timedout = true - @CompileManager.doCompile = sinon.stub().callsArgWith(1, @error, null) + @CompileManager.doCompileWithLock = sinon.stub().callsArgWith(1, @error, null) @CompileController.compile @req, @res it "should return the JSON response with the timeout status", -> @@ -118,7 +118,7 @@ describe "CompileController", -> describe "when the request returns no output files", -> beforeEach -> - @CompileManager.doCompile = sinon.stub().callsArgWith(1, null, []) + @CompileManager.doCompileWithLock = sinon.stub().callsArgWith(1, null, []) @CompileController.compile @req, @res it "should return the JSON response with the failure status", -> diff --git a/services/clsi/test/unit/coffee/CompileManagerTests.coffee b/services/clsi/test/unit/coffee/CompileManagerTests.coffee index ff671b27c3..b07a02cd85 100644 --- a/services/clsi/test/unit/coffee/CompileManagerTests.coffee +++ b/services/clsi/test/unit/coffee/CompileManagerTests.coffee @@ -19,9 +19,50 @@ describe "CompileManager", -> "./CommandRunner": @CommandRunner = {} "./DraftModeManager": @DraftModeManager = {} "./TikzManager": @TikzManager = {} + "./LockManager": @LockManager = {} "fs": @fs = {} @callback = sinon.stub() + describe "doCompileWithLock", -> + beforeEach -> + @request = + resources: @resources = "mock-resources" + project_id: @project_id = "project-id-123" + user_id: @user_id = "1234" + @output_files = ["foo", "bar"] + @CompileManager.doCompile = sinon.stub().callsArgWith(1, null, @output_files) + @LockManager.runWithLock = (lockFile, runner, callback) -> + runner (err, result...) -> + callback(err, result...) + + describe "when the project is not locked", -> + beforeEach -> + @CompileManager.doCompileWithLock @request, @callback + + it "should call doCompile with the request", -> + @CompileManager.doCompile + .calledWith(@request) + .should.equal true + + it "should call the callback with the output files", -> + @callback.calledWithExactly(null, @output_files) + .should.equal true + + describe "when the project is locked", -> + beforeEach -> + @error = new Error("locked") + @LockManager.runWithLock = (lockFile, runner, callback) => + callback(@error) + @CompileManager.doCompileWithLock @request, @callback + + it "should not call doCompile with the request", -> + @CompileManager.doCompile + .called.should.equal false + + it "should call the callback with the error", -> + @callback.calledWithExactly(@error) + .should.equal true + describe "doCompile", -> beforeEach -> @output_files = [{ diff --git a/services/clsi/test/unit/coffee/LockManager.coffee b/services/clsi/test/unit/coffee/LockManager.coffee new file mode 100644 index 0000000000..c1071a5841 --- /dev/null +++ b/services/clsi/test/unit/coffee/LockManager.coffee @@ -0,0 +1,54 @@ +SandboxedModule = require('sandboxed-module') +sinon = require('sinon') +require('chai').should() +modulePath = require('path').join __dirname, '../../../app/js/LockManager' +Path = require "path" +Errors = require "../../../app/js/Errors" + +describe "LockManager", -> + beforeEach -> + @LockManager = SandboxedModule.require modulePath, requires: + "settings-sharelatex": {} + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } + "lockfile": @Lockfile = {} + @lockFile = "/local/compile/directory/.project-lock" + + describe "runWithLock", -> + beforeEach -> + @runner = sinon.stub().callsArgWith(0, null, "foo", "bar") + @callback = sinon.stub() + + describe "normally", -> + beforeEach -> + @Lockfile.lock = sinon.stub().callsArgWith(2, null) + @Lockfile.unlock = sinon.stub().callsArgWith(1, null) + @LockManager.runWithLock @lockFile, @runner, @callback + + it "should run the compile", -> + @runner + .calledWith() + .should.equal true + + it "should call the callback with the response from the compile", -> + @callback + .calledWithExactly(null, "foo", "bar") + .should.equal true + + describe "when the project is locked", -> + beforeEach -> + @error = new Error() + @error.code = "EEXIST" + @Lockfile.lock = sinon.stub().callsArgWith(2,@error) + @Lockfile.unlock = sinon.stub().callsArgWith(1, null) + @LockManager.runWithLock @lockFile, @runner, @callback + + it "should not run the compile", -> + @runner + .called + .should.equal false + + it "should return an error", -> + error = new Errors.AlreadyCompilingError() + @callback + .calledWithExactly(error) + .should.equal true