diff --git a/lib/appenders/file.js b/lib/appenders/file.js index f2c194b..f14a716 100644 --- a/lib/appenders/file.js +++ b/lib/appenders/file.js @@ -5,6 +5,14 @@ const os = require('os'); const eol = os.EOL; +let mainSighupListenerStarted = false; +const sighupListeners = new Set(); +function mainSighupHandler() { + sighupListeners.forEach((app) => { + app.sighupHandler(); + }); +} + function openTheStream(file, fileSize, numFiles, options) { const stream = new streams.RollingFileStream( file, @@ -76,14 +84,22 @@ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset }; app.shutdown = function (complete) { - process.removeListener('SIGHUP', app.sighupHandler); + sighupListeners.delete(app); + if (sighupListeners.size === 0 && mainSighupListenerStarted) { + process.removeListener('SIGHUP', mainSighupHandler); + mainSighupListenerStarted = false; + } writer.end('', 'utf-8', complete); }; // On SIGHUP, close and reopen all files. This allows this appender to work with // logrotate. Note that if you are using logrotate, you should not set // `logSize`. - process.on('SIGHUP', app.sighupHandler); + sighupListeners.add(app); + if (!mainSighupListenerStarted) { + process.on('SIGHUP', mainSighupHandler); + mainSighupListenerStarted = true; + } return app; } diff --git a/test/tap/file-sighup-test.js b/test/tap/file-sighup-test.js index 3d565de..0627532 100644 --- a/test/tap/file-sighup-test.js +++ b/test/tap/file-sighup-test.js @@ -1,9 +1,68 @@ const { test } = require("tap"); +const path = require("path"); +const fs = require("fs"); const sandbox = require("@log4js-node/sandboxed-module"); +const removeFiles = async filenames => { + if (!Array.isArray(filenames)) + filenames = [filenames]; + const promises = filenames.map(filename => { + return fs.promises.unlink(filename); + }); + await Promise.allSettled(promises); +}; + +test("file appender single SIGHUP handler", t => { + const initialListeners = process.listenerCount("SIGHUP"); + + let warning; + const warningListener = error => { + if (error.type === "SIGHUP" && error.name === "MaxListenersExceededWarning") { + warning = error; + } + }; + process.on("warning", warningListener); + + const config = { + appenders: {}, + categories: { + default: { appenders: [], level: 'debug' } + } + }; + + // create 11 appenders to make nodejs warn for >10 max listeners + const numOfAppenders = 11; + for (let i = 1; i <= numOfAppenders; i++) { + config.appenders[`app${i}`] = { type: 'file', filename: path.join(__dirname, `file${i}.log`) }; + config.categories.default.appenders.push(`app${i}`); + } + + const log4js = require("../../lib/log4js"); + log4js.configure(config); + + t.teardown(async () => { + log4js.shutdown(); + + const filenames = Object.values(config.appenders).map(appender => { + return appender.filename; + }); + await removeFiles(filenames); + + process.off("warning", warningListener); + }); + + t.plan(2); + // put in a timeout 0 to allow event emitter/listener to happen + setTimeout(() => { + t.notOk(warning, "should not have MaxListenersExceededWarning for SIGHUP"); + t.equal(process.listenerCount("SIGHUP") - initialListeners, 1, "should be 1 SIGHUP listener"); + t.end(); + }, 0); +}); + // no SIGHUP signals on Windows, so don't run the tests if (process.platform !== "win32") { - + test("file appender SIGHUP", t => { let closeCalled = 0; let openCalled = 0;