diff --git a/Makefile b/Makefile index c87a8ea7f1cedf..cc20005baa4df3 100644 --- a/Makefile +++ b/Makefile @@ -387,7 +387,7 @@ bench-idle: ./$(NODE_EXE) benchmark/idle_clients.js & jslint: - ./$(NODE_EXE) tools/eslint/bin/eslint.js src/*.js lib/*.js --reset --quiet + ./$(NODE_EXE) tools/eslint/bin/eslint.js src/*.js lib/*.js lib/internal/*.js --reset --quiet CPPLINT_EXCLUDE ?= CPPLINT_EXCLUDE += src/node_lttng.cc diff --git a/lib/internal/repl.js b/lib/internal/repl.js index 902e81f495573c..97451821d74757 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -3,6 +3,9 @@ const Interface = require('readline').Interface; const REPL = require('repl'); const path = require('path'); +const util = require('util'); + +const debug = util.debuglog('repl'); module.exports = Object.create(REPL); module.exports.createInternalRepl = createRepl; @@ -17,16 +20,18 @@ function replStart() { return REPL.start.apply(REPL, arguments); } -function createRepl(env, cb) { +function createRepl(env, attemptPersistentHistory, cb) { const opts = { ignoreUndefined: false, - terminal: process.stdout.isTTY, useGlobal: true }; if (parseInt(env.NODE_NO_READLINE)) { opts.terminal = false; } + if (parseInt(env.NODE_FORCE_READLINE)) { + opts.terminal = true; + } if (parseInt(env.NODE_DISABLE_COLORS)) { opts.useColors = false; } @@ -51,8 +56,13 @@ function createRepl(env, cb) { } const repl = REPL.start(opts); - if (opts.terminal && env.NODE_REPL_HISTORY_FILE) { - return setupHistory(repl, env.NODE_REPL_HISTORY_FILE, cb); + if (env.NODE_REPL_HISTORY_FILE && attemptPersistentHistory) { + return setupHistory(repl, env.NODE_REPL_HISTORY_FILE, function(err) { + if (err) { + repl.close(); + } + cb(err, repl); + }); } repl._historyPrev = _replHistoryMessage; cb(null, repl); @@ -64,27 +74,14 @@ function setupHistory(repl, historyPath, ready) { var writing = false; var pending = false; repl.pause(); - fs.open(historyPath, 'a+', oninit); - - function oninit(err, hnd) { - if (err) { - return ready(err); - } - fs.close(hnd, onclose); - } - - function onclose(err) { - if (err) { - return ready(err); - } - fs.readFile(historyPath, 'utf8', onread); - } + fs.readFile(historyPath, {encoding: 'utf8', flag: 'a+'}, ondata); - function onread(err, data) { + function ondata(err, data) { if (err) { return ready(err); } + debug('loaded %s', historyPath); if (data) { try { repl.history = JSON.parse(data); @@ -98,17 +95,16 @@ function setupHistory(repl, historyPath, ready) { } } - fs.open(historyPath, 'w', onhandle); + getTempFile(ontmpfile); } - function onhandle(err, hnd) { + function ontmpfile(err, tmpinfo) { if (err) { return ready(err); } - repl._historyHandle = hnd; + repl._historyTempInfo = tmpinfo; repl.on('line', online); - - // reading the file data out erases it + repl.on('exit', removeTempFile); repl.once('flushHistory', function() { repl.resume(); ready(null, repl); @@ -134,11 +130,11 @@ function setupHistory(repl, historyPath, ready) { return; } writing = true; - const historyData = JSON.stringify(repl.history, null, 2); - fs.write(repl._historyHandle, historyData, 0, 'utf8', onwritten); + const historyData = JSON.stringify(repl.history || [], null, 2); + writeAndSwapTemp(historyData, repl._historyTempInfo, historyPath, onflushed); } - function onwritten(err, data) { + function onflushed(err, data) { writing = false; if (pending) { pending = false; @@ -150,6 +146,17 @@ function setupHistory(repl, historyPath, ready) { } } } + + function removeTempFile() { + if (repl._flushing) + return repl.once('flushHistory', function() { + removeTempFile(); + }); + repl._flushing = true; + fs.unlink(repl._historyTempInfo.path, function() { + onflushed(); + }); + } } @@ -165,3 +172,63 @@ function _replHistoryMessage() { this._historyPrev = Interface.prototype._historyPrev; return this._historyPrev(); } + + +function getTempFile(ready) { + var tmpPath = os.tmpdir(); + if (!tmpPath) { + return ready(new Error('no tmpdir available')); + } + tmpPath = path.join(tmpPath, `${process.pid}-node-repl.tmp`); + fs.open(tmpPath, 'wx', 0o600, function(err, fd) { + if (err) { + return ready(err); + } + return ready(null, { + fd: fd, + path: tmpPath + }); + }); +} + +// Write data, sync the fd, close it, overwrite the target +// with a rename, and open a new tmpfile. +// +// We use fs.write instead of writeFile because the latter +// does not accept an fd at the time of writing. +function writeAndSwapTemp(data, tmpInfo, target, ready) { + debug('writing tmp file... %s', tmpInfo.path, data); + return fs.write(tmpInfo.fd, data, 0, 'utf8', onwritten); + + function onwritten(err) { + if (err) return ready(err); + debug('syncing... %s', tmpInfo.path); + fs.fsync(tmpInfo.fd, onsync); + } + + function onsync(err) { + if (err) return ready(err); + debug('closing... %s', tmpInfo.path); + fs.close(tmpInfo.fd, onclose); + } + + function onclose(err) { + if (err) return ready(err); + debug('rename... %s -> %s', tmpInfo.path, target); + fs.rename(tmpInfo.path, target, onrename); + } + + function onrename(err) { + if (err) return ready(err); + debug('re-loading...'); + getTempFile(ontmpfile); + } + + function ontmpfile(err, info) { + if (err) return ready(err); + tmpInfo.fd = info.fd; + tmpInfo.path = info.path; + debug('done!'); + return ready(null); + } +} diff --git a/src/node.js b/src/node.js index 55b1d782ce82b0..258b51bacfe382 100644 --- a/src/node.js +++ b/src/node.js @@ -131,9 +131,20 @@ if (process._forceRepl || NativeModule.require('tty').isatty(0)) { // REPL var cliRepl = Module.requireRepl(); - cliRepl.createInternalRepl(process.env, function(err, repl) { + cliRepl.createInternalRepl(process.env, true, function(err, repl) { if (err) { - throw err; + console.error('Encountered error with persistent history support.') + console.error('Run with NODE_DEBUG=repl for more information.'); + if (/repl/.test(process.env.NODE_DEBUG || '')) { + console.error(err.stack); + } + return cliRepl.createInternalRepl( + process.env, false, function(err, repl) { + if (err) { + throw err; + } + repl.on('exit', process.exit); + }); } repl.on('exit', function() { if (repl._flushing) { diff --git a/test/parallel/test-internal-repl-race.js b/test/parallel/test-internal-repl-race.js new file mode 100644 index 00000000000000..dd31a9d413909b --- /dev/null +++ b/test/parallel/test-internal-repl-race.js @@ -0,0 +1,60 @@ +var spawn = require('child_process').spawn; +var common = require('../common'); +var assert = require('assert'); +var path = require('path'); +var os = require('os'); +var fs = require('fs'); + +var filename = path.join(common.tmpDir, 'node-history.json'); + +var config = { + env: { + NODE_REPL_HISTORY_FILE: filename, + NODE_FORCE_READLINE: 1 + } +} + +var lhs = spawn(process.execPath, ['-i'], config); +var rhs = spawn(process.execPath, ['-i'], config); + +lhs.stdout.pipe(process.stdout); +rhs.stdout.pipe(process.stdout); +lhs.stderr.pipe(process.stderr); +rhs.stderr.pipe(process.stderr); + + +var i = 0; +var tried = 0; +var exitcount = 0; +while (i++ < 100) { + lhs.stdin.write('hello = 0' + os.EOL); + rhs.stdin.write('OK = 0' + os.EOL); +} +lhs.stdin.write('\u0004') +rhs.stdin.write('\u0004') + +lhs.once('exit', onexit) +rhs.once('exit', onexit) + +function onexit() { + if (++exitcount !== 2) { + return; + } + check(); +} + +function check() { + fs.readFile(filename, 'utf8', function(err, data) { + if (err) { + console.log(err) + if (++tried > 100) { + throw err; + } + return setTimeout(check, 15); + } + assert.doesNotThrow(function() { + console.log(data) + JSON.parse(data); + }) + }) +}