Merge branch 'bg-lock-compiles'

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 423 Locked
+						status = "compile-in-progress"
+					else if error instanceof Errors.FilesOutOfSyncError
 						code = 409 # Http 409 Conflict
 						status = "retry"
 					else if error?.terminated

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)
 

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

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...)

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",

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", ->

services/clsi/test/unit/coffee/CompileManagerTests.coffee

@@ -19,9 +19,61 @@ describe "CompileManager", ->
 			"./CommandRunner": @CommandRunner = {}
 			"./DraftModeManager": @DraftModeManager = {}
 			"./TikzManager": @TikzManager = {}
+			"./LockManager": @LockManager = {}
 			"fs": @fs = {}
+			"fs-extra": @fse = { ensureDir: sinon.stub().callsArg(1) }
 		@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"]
+			@Settings.compileDir = "compiles"
+			@compileDir = "#{@Settings.path.compilesDir}/#{@project_id}-#{@user_id}"
+			@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 ensure that the compile directory exists", ->
+				@fse.ensureDir.calledWith(@compileDir)
+				.should.equal true
+
+			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 ensure that the compile directory exists", ->
+				@fse.ensureDir.calledWith(@compileDir)
+				.should.equal true
+
+			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 = [{

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