diff --git a/CHANGES b/CHANGES index e64d2d105..35ca0c012 100644 --- a/CHANGES +++ b/CHANGES @@ -2239,4 +2239,5 @@ * addons: better error handling * Fix bug where renew certs button did not work * filemanager: various enhancements +* sftp: fix rebuild condition diff --git a/src/apptaskmanager.js b/src/apptaskmanager.js index 1482daa9e..74eb9e884 100644 --- a/src/apptaskmanager.js +++ b/src/apptaskmanager.js @@ -79,14 +79,16 @@ function scheduleTask(appId, taskId, options, callback) { delete gActiveTasks[appId]; locker.unlock(locker.OP_APPTASK); // unlock event will trigger next task - // post app task hooks - services.rebuildService('sftp', error => { if (error) debug('Unable to rebuild sftp:', error); }); scheduler.resumeJobs(appId); }); } function startNextTask() { - if (gPendingTasks.length === 0) return; + if (gPendingTasks.length === 0) { + // rebuild sftp when task queue is empty. this minimizes risk of sftp rebuild overlapping with other app tasks + services.rebuildService('sftp', error => { if (error) debug('Unable to rebuild sftp:', error); }); + return; + } assert(Object.keys(gActiveTasks).length < TASK_CONCURRENCY); diff --git a/src/sftp.js b/src/sftp.js index b1303c4ef..6ca7a4680 100644 --- a/src/sftp.js +++ b/src/sftp.js @@ -21,24 +21,11 @@ var apps = require('./apps.js'), volumes = require('./volumes.js'), _ = require('underscore'); -var gRebuildInProgress = false; function rebuild(serviceConfig, options, callback) { assert.strictEqual(typeof serviceConfig, 'object'); assert.strictEqual(typeof options, 'object'); assert.strictEqual(typeof callback, 'function'); - if (gRebuildInProgress) { - debug('waiting for other rebuild to finish'); - return setTimeout(function () { rebuild(serviceConfig, options, callback); }, 5000); - } - - gRebuildInProgress = true; - - function done(error) { - gRebuildInProgress = false; - callback(error); - } - debug('rebuilding container'); const force = !!options.force; @@ -48,7 +35,7 @@ function rebuild(serviceConfig, options, callback) { const cloudronToken = hat(8 * 128); apps.getAll(function (error, result) { - if (error) return done(error); + if (error) return callback(error); let dataDirs = []; result.forEach(function (app) { @@ -89,7 +76,7 @@ function rebuild(serviceConfig, options, callback) { if (!force && _.isEqual(currentDataDirs, dataDirs)) { debug('Skipping rebuild, no changes'); - return done(); + return callback(); } } } @@ -119,7 +106,7 @@ function rebuild(serviceConfig, options, callback) { shell.exec.bind(null, 'stopSftp', 'docker stop sftp || true'), shell.exec.bind(null, 'removeSftp', 'docker rm -f sftp || true'), shell.exec.bind(null, 'startSftp', cmd) - ], done); + ], callback); }); }); });