fix race condition where initializing REPL history could cause Lumo t…

…o read file

size of inexistent file

CHANGELOG.md

@@ -11,6 +11,8 @@
 - Fix regression that prevented requiring binary modules ([#163](https://github.com/anmonteiro/lumo/issues/163)).
 - Fix bug that prevented Lumo from continuing executing after being put in background ([#166](https://github.com/anmonteiro/lumo/issues/166)).
 - Fix regression that prevented requiring foreign libraries ([#167](https://github.com/anmonteiro/lumo/issues/167)).
+- Fix a race condition with the REPL history where Lumo could attempt to read a file's
+size before it was created.
 
 ## [1.5.0](https://github.com/anmonteiro/lumo/compare/1.4.1...1.5.0) (2017-05-13)
 

src/js/__tests__/replHistory-test.js

@@ -26,8 +26,10 @@ fs.unlink = jest.fn((_: string, cb: () => void) => cb());
 const streamWrite = jest.fn();
 
 fs.createWriteStream = jest.fn(() => ({
+  on(_: string, cb: number => void): void {
+    cb(42);
+  },
   write: streamWrite,
-  fd: 42,
 }));
 
 fs.createReadStream = jest.fn(

src/js/replHistory.js

@@ -16,27 +16,14 @@ type replHistory$Opts = {
 };
 
 const maxDiskSize = 0x1000000;
-let histStream;
-
-function getHistoryStream(path: string): stream$Writable {
-  if (histStream == null) {
-    histStream = fs.createWriteStream(path, {
-      flags: 'a+',
-      defaultEncoding: 'utf8',
-    });
-  }
-
-  return histStream;
-}
 
 function onLoad(
   path: string,
+  fd: number,
   maxLength: number,
   offset: number,
   cb: (ret: string[]) => void,
 ): void {
-  // $FlowIssue: it's there
-  const { fd } = getHistoryStream(path);
   const rs = fs.createReadStream(path, {
     encoding: 'utf-8',
     fd,
@@ -57,7 +44,7 @@ function onLoad(
 
     if (offset > 0 && tail.length < maxLength) {
       // eslint-disable-next-line no-mixed-operators
-      onLoad(path, maxLength, offset - 0x100 * maxLength, cb);
+      onLoad(path, fd, maxLength, offset - 0x100 * maxLength, cb);
     } else {
       tail = tail.slice(-maxLength);
       tail.reverse();
@@ -68,6 +55,7 @@ function onLoad(
 
 function loadHistory(
   path: string,
+  fd: number,
   maxLength: number,
   cb: (ret: string[]) => void,
 ): void {
@@ -77,15 +65,12 @@ function loadHistory(
       const rename = function rename(): void {
         fs.rename(path, `${path}.old`, () => {
           // $FlowIssue: mode is optional
-          fs.open(path, 'a+', (e: ?ErrnoError, fd: number) => {
-            loadHistory(path, maxLength, cb);
+          fs.open(path, 'a+', (e: ?ErrnoError, ffd: number) => {
+            loadHistory(path, ffd, maxLength, cb);
           });
         });
       };
 
-      // $FlowIssue: it's there
-      const { fd } = getHistoryStream(path);
-
       if (fd != null) {
         fs.close(fd, () => {
           const oldPath = `${path}.old`;
@@ -99,7 +84,7 @@ function loadHistory(
         });
       }
     } else {
-      onLoad(path, maxLength, totalSize, cb);
+      onLoad(path, fd, maxLength, totalSize, cb);
     }
   });
 }
@@ -112,7 +97,10 @@ export default function createInterface(
   const rl = readline.createInterface(options);
 
   if (terminal) {
-    const stream = getHistoryStream(path);
+    const stream = fs.createWriteStream(path, {
+      flags: 'a+',
+      defaultEncoding: 'utf8',
+    });
 
     // $FlowIssue: private property
     const oldAddHistory = rl._addHistory;
@@ -130,9 +118,11 @@ export default function createInterface(
     };
 
     if (path != null && historySize != null) {
-      loadHistory(path, historySize, (history: string[]) => {
-        // $FlowIssue: it's there
-        rl.history.push(...history);
+      stream.on('open', (fd: number) => {
+        loadHistory(path, fd, historySize, (history: string[]) => {
+          // $FlowIssue: it's there
+          rl.history.push(...history);
+        });
       });
     }
   }