diff --git a/services/spelling/Gruntfile.coffee b/services/spelling/Gruntfile.coffee index 9bf2f98681..038057c3ad 100644 --- a/services/spelling/Gruntfile.coffee +++ b/services/spelling/Gruntfile.coffee @@ -31,6 +31,8 @@ module.exports = (grunt) -> unit: options: reporter: grunt.option('reporter') or 'spec' + grep: grunt.option("grep") + timeout: grunt.option("timeout") src: ["test/unit/js/**/*.js"] grunt.loadNpmTasks 'grunt-contrib-coffee' diff --git a/services/spelling/app/coffee/ASpell.coffee b/services/spelling/app/coffee/ASpell.coffee index 92437ebecf..392b543706 100644 --- a/services/spelling/app/coffee/ASpell.coffee +++ b/services/spelling/app/coffee/ASpell.coffee @@ -1,5 +1,4 @@ async = require "async" -_ = require "underscore" ASpellWorkerPool = require "./ASpellWorkerPool" LRU = require "lru-cache" logger = require 'logger-sharelatex' @@ -87,7 +86,6 @@ module.exports = ASpell = # http://aspell.net/man-html/Through-A-Pipe.html checkWords: (language, words, callback = (error, result) ->) -> runner = new ASpellRunner() - callback = _.once callback runner.checkWords language, words, callback ASPELL_TIMEOUT : 4000 diff --git a/services/spelling/app/coffee/ASpellWorker.coffee b/services/spelling/app/coffee/ASpellWorker.coffee index 168be52ee6..05e4a93adb 100644 --- a/services/spelling/app/coffee/ASpellWorker.coffee +++ b/services/spelling/app/coffee/ASpellWorker.coffee @@ -1,6 +1,7 @@ child_process = require("child_process") logger = require 'logger-sharelatex' metrics = require('metrics-sharelatex') +_ = require "underscore" BATCH_SIZE = 100 @@ -15,16 +16,24 @@ class ASpellWorker @state = 'killed' logger.log process: @pipe.pid, lang: @language, "aspell worker has exited" metrics.inc "aspellWorker-exit-" + @language + @pipe.on 'close', () => + @state = 'closed' unless @state == 'killed' + if @callback? + logger.err process: @pipe.pid, lang: @language, "aspell worker closed output streams with uncalled callback" + @callback new Error("aspell worker closed output streams with uncalled callback"), [] + @callback = null @pipe.on 'error', (err) => @state = 'error' unless @state == 'killed' logger.log process: @pipe.pid, error: err, stdout: output.slice(-1024), stderr: error.slice(-1024), lang: @language, "aspell worker error" if @callback? @callback err, [] + @callback = null @pipe.stdin.on 'error', (err) => @state = 'error' unless @state == 'killed' logger.log process: @pipe.pid, error: err, stdout: output.slice(-1024), stderr: error.slice(-1024), lang: @language, "aspell worker error on stdin" if @callback? @callback err, [] + @callback = null output = "" endMarker = new RegExp("^[a-z][a-z]", "m") @@ -33,6 +42,7 @@ class ASpellWorker # We receive the language code from Aspell as the end of data marker if chunk.toString().match(endMarker) @callback(null, output.slice()) + @callback = null # only allow one callback in use @state = 'ready' output = "" error = "" @@ -52,7 +62,10 @@ class ASpellWorker # we will now send data to aspell, and be ready again when we # receive the end of data marker @state = 'busy' - @callback = callback + if @callback? # only allow one callback in use + logger.err process: @pipe.pid, lang: @language, "CALLBACK ALREADY IN USE" + return @callback new Error("Aspell callback already in use - SHOULD NOT HAPPEN") + @callback = _.once callback # extra defence against double callback @setTerseMode() @write(words) @flush() diff --git a/services/spelling/test/unit/coffee/ASpellTests.coffee b/services/spelling/test/unit/coffee/ASpellTests.coffee index 1170f6baa1..742a5aadbb 100644 --- a/services/spelling/test/unit/coffee/ASpellTests.coffee +++ b/services/spelling/test/unit/coffee/ASpellTests.coffee @@ -6,7 +6,10 @@ assert = require("chai").assert describe "ASpell", -> beforeEach -> - @ASpell = SandboxedModule.require "../../../app/js/ASpell", requires:{} + @ASpell = SandboxedModule.require "../../../app/js/ASpell", requires: + "logger-sharelatex": + log:-> + err:-> describe "a correctly spelled word", -> beforeEach (done) -> @@ -58,7 +61,6 @@ describe "ASpell", -> # Note that this test fails on OS X, due to differing pipe behaviour # on killing the child process. It can be tested successfully on Travis # or the CI server. - it "should return in reasonable time", (done) -> + it "should return in reasonable time", () -> delta = Date.now()-@start - delta.should.be.below(@ASpell.ASPELL_TIMEOUT + 100) - done() + delta.should.be.below(@ASpell.ASPELL_TIMEOUT + 1000) diff --git a/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee b/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee index 984beb37b8..6cbe6af9c9 100644 --- a/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee +++ b/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee @@ -19,6 +19,10 @@ describe "LearnedWordsManager", -> @LearnedWordsManager = SandboxedModule.require modulePath, requires: "./DB" : @db "./MongoCache":@cache + "logger-sharelatex": + log:-> + err:-> + info:-> describe "learnWord", -> beforeEach ->