From 5140fff347030bccc8e0440077018220e4441d8a Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Tue, 4 Nov 2025 13:01:40 +0100 Subject: [PATCH] [clsi] gracefully handle fast exit of synctex/wordcount containers (#29505) * [clsi] gracefully handle fast exit of synctex/wordcount containers * [clsi] do not change container options in-place for logging GitOrigin-RevId: 0b685310a3c72f8f46125fefaa30c1ddb19e7b07 --- services/clsi/app/js/DockerRunner.js | 60 ++++++++++++------- .../clsi/test/unit/js/DockerRunnerTests.js | 47 ++++++++++++++- 2 files changed, 83 insertions(+), 24 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 77622c46a2..113b4baf37 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -176,29 +176,35 @@ const DockerRunner = { return callback(error) } - DockerRunner.waitForContainer(name, timeout, (error, exitCode) => { - if (error != null) { - return callback(error) + DockerRunner.waitForContainer( + name, + timeout, + options, + (error, exitCode) => { + if (error != null) { + return callback(error) + } + if (exitCode === 137) { + // exit status from kill -9 + const err = new Error('terminated') + err.terminated = true + return callback(err) + } + if (exitCode === 1) { + // exit status from chktex + const err = new Error('exited') + err.code = exitCode + return callback(err) + } + containerReturned = true + logger.debug( + // The seccomp policy is very large. Avoid logging it. _.omit deep clones. + { exitCode, options: _.omit(options, 'HostConfig.SecurityOpt') }, + 'docker container has exited' + ) + callbackIfFinished() } - if (exitCode === 137) { - // exit status from kill -9 - const err = new Error('terminated') - err.terminated = true - return callback(err) - } - if (exitCode === 1) { - // exit status from chktex - const err = new Error('exited') - err.code = exitCode - return callback(err) - } - containerReturned = true - if (options != null && options.HostConfig != null) { - options.HostConfig.SecurityOpt = null - } - logger.debug({ exitCode, options }, 'docker container has exited') - callbackIfFinished() - }) + ) } ) }, @@ -429,7 +435,7 @@ const DockerRunner = { }) }, - waitForContainer(containerId, timeout, _callback) { + waitForContainer(containerId, timeout, options, _callback) { const callback = _.once(_callback) const container = dockerode.getContainer(containerId) @@ -445,6 +451,14 @@ const DockerRunner = { logger.debug({ containerId }, 'waiting for docker container') container.wait((error, res) => { + if (error?.statusCode === 404 && options.HostConfig.AutoRemove) { + logger.debug( + { containerId }, + 'auto-destroy container destroyed before starting to wait' + ) + clearTimeout(timeoutId) + return callback(null, 0) + } if (error != null) { clearTimeout(timeoutId) logger.warn({ err: error, containerId }, 'error waiting for container') diff --git a/services/clsi/test/unit/js/DockerRunnerTests.js b/services/clsi/test/unit/js/DockerRunnerTests.js index 2ef0a3c072..ea82810d60 100644 --- a/services/clsi/test/unit/js/DockerRunnerTests.js +++ b/services/clsi/test/unit/js/DockerRunnerTests.js @@ -514,7 +514,7 @@ describe('DockerRunner', function () { sinon.spy(this.DockerRunner, 'startContainer') this.DockerRunner.waitForContainer = sinon .stub() - .callsArgWith(2, null, (this.exitCode = 42)) + .callsArgWith(3, null, (this.exitCode = 42)) return this.DockerRunner._runAndWaitForContainer( this.options, this.volumes, @@ -675,6 +675,7 @@ describe('DockerRunner', function () { return this.DockerRunner.waitForContainer( this.containerId, this.timeout, + {}, this.callback ) }) @@ -691,6 +692,49 @@ describe('DockerRunner', function () { }) }) + describe('when the container is removed before waiting', function () { + const err = new Error('not found') + err.statusCode = 404 + beforeEach(function () { + this.container.wait = sinon.stub().yields(err) + }) + + describe('AutoRemove not set', function () { + beforeEach(function () { + this.DockerRunner.waitForContainer( + this.containerId, + this.timeout, + { HostConfig: {} }, + this.callback + ) + }) + it('should wait for the container', function () { + this.getContainer.calledWith(this.containerId).should.equal(true) + this.container.wait.called.should.equal(true) + }) + it('should call the callback with the error', function () { + this.callback.calledWith(err).should.equal(true) + }) + }) + describe('AutoRemove=true', function () { + beforeEach(function () { + this.DockerRunner.waitForContainer( + this.containerId, + this.timeout, + { HostConfig: { AutoRemove: true } }, + this.callback + ) + }) + it('should wait for the container', function () { + this.getContainer.calledWith(this.containerId).should.equal(true) + this.container.wait.called.should.equal(true) + }) + it('should call the callback with exit code 0', function () { + this.callback.calledWith(null, 0).should.equal(true) + }) + }) + }) + return describe('when the container does not return before the timeout', function () { beforeEach(function (done) { this.container.wait = function (callback) { @@ -703,6 +747,7 @@ describe('DockerRunner', function () { return this.DockerRunner.waitForContainer( this.containerId, this.timeout, + {}, (...args) => { this.callback(...Array.from(args || [])) return done()