diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index b4ef1e525..0a4f95181 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -10,6 +10,28 @@ Things in context: Use this hook to receive the global settings in your plugin. +## shutdown +Called from: src/node/server.js + +Things in context: None + +This hook runs before shutdown. Use it to stop timers, close sockets and files, +flush buffers, etc. The database is not available while this hook is running. +The shutdown function must not block for long because there is a short timeout +before the process is forcibly terminated. + +The shutdown function must return a Promise, which must resolve to `undefined`. +Returning `callback(value)` will return a Promise that is resolved to `value`. + +Example: + +``` +// using an async function +exports.shutdown = async (hookName, context) => { + await flushBuffers(); +}; +``` + ## pluginUninstall Called from: src/static/js/pluginfw/installer.js diff --git a/src/ep.json b/src/ep.json index 428f57269..b436a63be 100644 --- a/src/ep.json +++ b/src/ep.json @@ -1,10 +1,14 @@ { "parts": [ + { "name": "DB", "hooks": { "shutdown": "ep_etherpad-lite/node/db/DB" } }, + { "name": "Minify", "hooks": { "shutdown": "ep_etherpad-lite/node/utils/Minify" } }, { "name": "express", "hooks": { - "createServer": "ep_etherpad-lite/node/hooks/express:createServer", - "restartServer": "ep_etherpad-lite/node/hooks/express:restartServer" + "createServer": "ep_etherpad-lite/node/hooks/express", + "restartServer": "ep_etherpad-lite/node/hooks/express", + "shutdown": "ep_etherpad-lite/node/hooks/express" } }, { "name": "static", "hooks": { "expressCreateServer": "ep_etherpad-lite/node/hooks/express/static:expressCreateServer" } }, + { "name": "stats", "hooks": { "shutdown": "ep_etherpad-lite/node/stats" } }, { "name": "i18n", "hooks": { "expressCreateServer": "ep_etherpad-lite/node/hooks/i18n:expressCreateServer" } }, { "name": "specialpages", "hooks": { "expressCreateServer": "ep_etherpad-lite/node/hooks/express/specialpages:expressCreateServer" } }, { "name": "padurlsanitize", "hooks": { "expressCreateServer": "ep_etherpad-lite/node/hooks/express/padurlsanitize:expressCreateServer" } }, diff --git a/src/node/db/DB.js b/src/node/db/DB.js index 4150743de..22c3635ea 100644 --- a/src/node/db/DB.js +++ b/src/node/db/DB.js @@ -71,3 +71,8 @@ exports.init = function() { }); }); } + +exports.shutdown = async (hookName, context) => { + await exports.doShutdown(); + console.log('Database closed'); +}; diff --git a/src/node/hooks/express.js b/src/node/hooks/express.js index 7ff7d4ffc..cb536ef4d 100644 --- a/src/node/hooks/express.js +++ b/src/node/hooks/express.js @@ -5,18 +5,20 @@ var fs = require('fs'); var path = require('path'); var npm = require("npm/lib/npm.js"); var _ = require("underscore"); +const util = require('util'); -var server; var serverName; -exports.createServer = function () { +exports.server = null; + +exports.createServer = async () => { console.log("Report bugs at https://github.com/ether/etherpad-lite/issues") serverName = `Etherpad ${settings.getGitCommit()} (https://etherpad.org)`; console.log(`Your Etherpad version is ${settings.getEpVersion()} (${settings.getGitCommit()})`); - exports.restartServer(); + await exports.restartServer(); if (settings.ip === "") { // using Unix socket for connectivity @@ -38,10 +40,10 @@ exports.createServer = function () { } } -exports.restartServer = function () { - if (server) { +exports.restartServer = async () => { + if (exports.server) { console.log("Restarting express server"); - server.close(); + await util.promisify(exports.server.close).bind(exports.server)(); } var app = express(); // New syntax for express v3 @@ -65,10 +67,10 @@ exports.restartServer = function () { } var https = require('https'); - server = https.createServer(options, app); + exports.server = https.createServer(options, app); } else { var http = require('http'); - server = http.createServer(app); + exports.server = http.createServer(app); } app.use(function(req, res, next) { @@ -110,7 +112,12 @@ exports.restartServer = function () { } hooks.callAll("expressConfigure", {"app": app}); - hooks.callAll("expressCreateServer", {"app": app, "server": server}); + hooks.callAll('expressCreateServer', {app, server: exports.server}); - server.listen(settings.port, settings.ip); -} + await util.promisify(exports.server.listen).bind(exports.server)(settings.port, settings.ip); +}; + +exports.shutdown = async (hookName, context) => { + if (!exports.server) return; + await util.promisify(exports.server.close).bind(exports.server)(); +}; diff --git a/src/node/hooks/express/adminsettings.js b/src/node/hooks/express/adminsettings.js index 1e0d6004f..1ad48b111 100644 --- a/src/node/hooks/express/adminsettings.js +++ b/src/node/hooks/express/adminsettings.js @@ -46,11 +46,10 @@ exports.socketio = function (hook_name, args, cb) { }); }); - socket.on("restartServer", function () { + socket.on('restartServer', async () => { console.log("Admin request to restart server through a socket on /admin/settings"); settings.reloadSettings(); - hooks.aCallAll("restartServer", {}, function () {}); - + await hooks.aCallAll('restartServer'); }); }); diff --git a/src/node/hooks/express/errorhandling.js b/src/node/hooks/express/errorhandling.js index 66553621c..d4b8b89dd 100644 --- a/src/node/hooks/express/errorhandling.js +++ b/src/node/hooks/express/errorhandling.js @@ -1,39 +1,5 @@ -var os = require("os"); -var db = require('../../db/DB'); var stats = require('ep_etherpad-lite/node/stats') - -exports.onShutdown = false; -exports.gracefulShutdown = function(err) { - if(err && err.stack) { - console.error(err.stack); - } else if(err) { - console.error(err); - } - - // ensure there is only one graceful shutdown running - if (exports.onShutdown) { - return; - } - - exports.onShutdown = true; - - console.log("graceful shutdown..."); - - // do the db shutdown - db.doShutdown().then(function() { - console.log("db sucessfully closed."); - - process.exit(0); - }); - - setTimeout(function() { - process.exit(1); - }, 3000); -} - -process.on('uncaughtException', exports.gracefulShutdown); - exports.expressCreateServer = function (hook_name, args, cb) { exports.app = args.app; @@ -46,28 +12,4 @@ exports.expressCreateServer = function (hook_name, args, cb) { console.error(err.stack? err.stack : err.toString()); stats.meter('http500').mark() }); - - /* - * Connect graceful shutdown with sigint and uncaught exception - * - * Until Etherpad 1.7.5, process.on('SIGTERM') and process.on('SIGINT') were - * not hooked up under Windows, because old nodejs versions did not support - * them. - * - * According to nodejs 6.x documentation, it is now safe to do so. This - * allows to gracefully close the DB connection when hitting CTRL+C under - * Windows, for example. - * - * Source: https://nodejs.org/docs/latest-v6.x/api/process.html#process_signal_events - * - * - SIGTERM is not supported on Windows, it can be listened on. - * - SIGINT from the terminal is supported on all platforms, and can usually - * be generated with +C (though this may be configurable). It is not - * generated when terminal raw mode is enabled. - */ - process.on('SIGINT', exports.gracefulShutdown); - - // when running as PID1 (e.g. in docker container) - // allow graceful shutdown on SIGTERM c.f. #3265 - process.on('SIGTERM', exports.gracefulShutdown); } diff --git a/src/node/server.js b/src/node/server.js index a1f62df4f..c9ef33cc9 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -21,65 +21,112 @@ * limitations under the License. */ -const log4js = require('log4js') - , NodeVersion = require('./utils/NodeVersion') - , UpdateCheck = require('./utils/UpdateCheck') - ; - +const log4js = require('log4js'); log4js.replaceConsole(); /* * early check for version compatibility before calling * any modules that require newer versions of NodeJS */ +const NodeVersion = require('./utils/NodeVersion'); NodeVersion.enforceMinNodeVersion('10.13.0'); - -/* - * Etherpad 1.8.3 will require at least nodejs 10.13.0. - */ NodeVersion.checkDeprecationStatus('10.13.0', '1.8.3'); -// Check if Etherpad version is up-to-date -UpdateCheck.check(); +const UpdateCheck = require('./utils/UpdateCheck'); +const db = require('./db/DB'); +const express = require('./hooks/express'); +const hooks = require('ep_etherpad-lite/static/js/pluginfw/hooks'); +const npm = require('npm/lib/npm.js'); +const plugins = require('ep_etherpad-lite/static/js/pluginfw/plugins'); +const settings = require('./utils/Settings'); +const util = require('util'); -/* - * start up stats counting system - */ -var stats = require('./stats'); -stats.gauge('memoryUsage', function() { - return process.memoryUsage().rss; -}); +let started = false; +let stopped = false; -/* - * no use of let or await here because it would cause startup - * to fail completely on very early versions of NodeJS - */ -var npm = require("npm/lib/npm.js"); +exports.start = async () => { + if (started) return; + started = true; + if (stopped) throw new Error('restart not supported'); -npm.load({}, function() { - var settings = require('./utils/Settings'); - var db = require('./db/DB'); - var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins"); - var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks"); + // Check if Etherpad version is up-to-date + UpdateCheck.check(); - db.init() - .then(plugins.update) - .then(function() { - console.info("Installed plugins: " + plugins.formatPluginsWithVersion()); - console.debug("Installed parts:\n" + plugins.formatParts()); - console.debug("Installed hooks:\n" + plugins.formatHooks()); + // start up stats counting system + const stats = require('./stats'); + stats.gauge('memoryUsage', () => process.memoryUsage().rss); - // Call loadSettings hook - hooks.aCallAll("loadSettings", { settings: settings }); + await util.promisify(npm.load)(); - // initalize the http server - hooks.callAll("createServer", {}); - }) - .catch(function(e) { - console.error("exception thrown: " + e.message); - if (e.stack) { - console.log(e.stack); - } - process.exit(1); - }); -}); + try { + await db.init(); + await plugins.update(); + console.info('Installed plugins: ' + plugins.formatPluginsWithVersion()); + console.debug('Installed parts:\n' + plugins.formatParts()); + console.debug('Installed hooks:\n' + plugins.formatHooks()); + await hooks.aCallAll('loadSettings', {settings}); + await hooks.aCallAll('createServer'); + } catch (e) { + console.error('exception thrown: ' + e.message); + if (e.stack) console.log(e.stack); + process.exit(1); + } + + process.on('uncaughtException', exports.exit); + + /* + * Connect graceful shutdown with sigint and uncaught exception + * + * Until Etherpad 1.7.5, process.on('SIGTERM') and process.on('SIGINT') were + * not hooked up under Windows, because old nodejs versions did not support + * them. + * + * According to nodejs 6.x documentation, it is now safe to do so. This + * allows to gracefully close the DB connection when hitting CTRL+C under + * Windows, for example. + * + * Source: https://nodejs.org/docs/latest-v6.x/api/process.html#process_signal_events + * + * - SIGTERM is not supported on Windows, it can be listened on. + * - SIGINT from the terminal is supported on all platforms, and can usually + * be generated with +C (though this may be configurable). It is not + * generated when terminal raw mode is enabled. + */ + process.on('SIGINT', exports.exit); + + // When running as PID1 (e.g. in docker container) allow graceful shutdown on SIGTERM c.f. #3265. + // Pass undefined to exports.exit because this is not an abnormal termination. + process.on('SIGTERM', () => exports.exit()); + + // Return the HTTP server to make it easier to write tests. + return express.server; +}; + +exports.stop = async () => { + if (stopped) return; + stopped = true; + console.log('Stopping Etherpad...'); + await new Promise(async (resolve, reject) => { + const id = setTimeout(() => reject(new Error('Timed out waiting for shutdown tasks')), 3000); + await hooks.aCallAll('shutdown'); + clearTimeout(id); + resolve(); + }); +}; + +exports.exit = async (err) => { + let exitCode = 0; + if (err) { + exitCode = 1; + console.error(err.stack ? err.stack : err); + } + try { + await exports.stop(); + } catch (err) { + exitCode = 1; + console.error(err.stack ? err.stack : err); + } + process.exit(exitCode); +}; + +if (require.main === module) exports.start(); diff --git a/src/node/stats.js b/src/node/stats.js index ff1752fe9..13654bb7d 100644 --- a/src/node/stats.js +++ b/src/node/stats.js @@ -1,3 +1,7 @@ var measured = require('measured-core') module.exports = measured.createCollection(); + +module.exports.shutdown = async (hookName, context) => { + module.exports.end(); +}; diff --git a/src/node/utils/Minify.js b/src/node/utils/Minify.js index a4194eb9e..e94d34b04 100644 --- a/src/node/utils/Minify.js +++ b/src/node/utils/Minify.js @@ -418,3 +418,7 @@ exports.minify = minify; exports.requestURI = requestURI; exports.requestURIs = requestURIs; + +exports.shutdown = async (hookName, context) => { + await threadsPool.terminate(); +}; diff --git a/src/package.json b/src/package.json index b01e07a02..b2d87f40a 100644 --- a/src/package.json +++ b/src/package.json @@ -92,7 +92,7 @@ "url": "https://github.com/ether/etherpad-lite.git" }, "scripts": { - "test": "nyc wtfnode node_modules/.bin/_mocha --timeout 5000 --recursive ../tests/backend/specs ../node_modules/ep_*/static/tests/backend/specs", + "test": "nyc wtfnode node_modules/.bin/_mocha --timeout 30000 --recursive ../tests/backend/specs ../node_modules/ep_*/static/tests/backend/specs", "test-container": "nyc mocha --timeout 5000 ../tests/container/specs/api" }, "version": "1.8.6", diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js index 9db6bdbeb..ba639a79c 100644 --- a/tests/backend/specs/socketio.js +++ b/tests/backend/specs/socketio.js @@ -1,51 +1,30 @@ function m(mod) { return __dirname + '/../../../src/' + mod; } const assert = require('assert').strict; -const db = require(m('node/db/DB')); -const express = require(m('node_modules/express')); -const http = require('http'); +const io = require(m('node_modules/socket.io-client')); const log4js = require(m('node_modules/log4js')); -let padManager; +const padManager = require(m('node/db/PadManager')); const plugins = require(m('static/js/pluginfw/plugin_defs')); +const server = require(m('node/server')); const setCookieParser = require(m('node_modules/set-cookie-parser')); const settings = require(m('node/utils/Settings')); -const io = require(m('node_modules/socket.io-client')); -const stats = require(m('node/stats')); const supertest = require(m('node_modules/supertest')); -const util = require('util'); const logger = log4js.getLogger('test'); -const app = express(); -const server = http.createServer(app); let client; let baseUrl; before(async () => { - await util.promisify(server.listen).bind(server)(0, 'localhost'); - baseUrl = `http://localhost:${server.address().port}`; + settings.port = 0; + settings.ip = 'localhost'; + const httpServer = await server.start(); + baseUrl = `http://localhost:${httpServer.address().port}`; logger.debug(`HTTP server at ${baseUrl}`); client = supertest(baseUrl); - const npm = require(m('node_modules/npm/lib/npm.js')); - await util.promisify(npm.load)(); - settings.users = { - admin: {password: 'admin-password', is_admin: true}, - user: {password: 'user-password'}, - }; - await db.init(); - padManager = require(m('node/db/PadManager')); - const webaccess = require(m('node/hooks/express/webaccess')); - webaccess.expressConfigure('expressConfigure', {app}); - const socketio = require(m('node/hooks/express/socketio')); - socketio.expressCreateServer('expressCreateServer', {app, server}); - app.get(/./, (req, res) => { res.status(200).send('OK'); }); }); after(async () => { - stats.end(); - await Promise.all([ - db.doShutdown(), - util.promisify(server.close).bind(server)(), - ]); + await server.stop(); }); // Waits for and returns the next named socket.io event. Rejects if there is any error while waiting @@ -129,16 +108,23 @@ const handshake = async (socket, padID) => { }; describe('socket.io access checks', () => { + const settingsBackup = {}; let socket; beforeEach(async () => { + Object.assign(settingsBackup, settings); assert(socket == null); settings.requireAuthentication = false; settings.requireAuthorization = false; + settings.users = { + admin: {password: 'admin-password', is_admin: true}, + user: {password: 'user-password'}, + }; Promise.all(['pad', 'other-pad'].map(async (pad) => { if (await padManager.doesPadExist(pad)) (await padManager.getPad(pad)).remove(); })); }); afterEach(async () => { + Object.assign(settings, settingsBackup); if (socket) socket.close(); socket = null; });