Fix dup*() syscalls

Rework dup syscalls so file descriptors share all flags and also seek position.

Author: socketpair

src/library_fs.js

@@ -400,10 +400,10 @@ mergeInto(LibraryManager.library, {
     getStream: function(fd) {
       return FS.streams[fd];
     },
-    // TODO parameterize this function such that a stream
-    // object isn't directly passed in. not possible until
-    // SOCKFS is completed.
-    createStream: function(stream, fd_start, fd_end) {
+    duplicateStream: function(stream) {
+      // Some properties are shared between duplicates
+      var sharedProperties = ["position", "flags", "refcount"];
+
       if (!FS.FSStream) {
         FS.FSStream = function(){};
         FS.FSStream.prototype = {};
@@ -421,15 +421,64 @@ mergeInto(LibraryManager.library, {
           },
           isAppend: {
             get: function() { return (this.flags & {{{ cDefine('O_APPEND') }}}); }
-          }
+          },
+          flags: {
+            get: function() { return this.shared.flags; },
+            set: function(value) { this.shared.flags = value; }
+          },
+          position: {
+            get: function() { return this.shared.position; },
+            set: function(value) { this.shared.position = value; }
+          },
+#if NODERAWFS
+          /* refcount is a stream attribute only used by NODERAWFS
+           *
+           * Duplicated streams share flags and seek position, as described in dup(2):
+           *
+           *   http://man7.org/linux/man-pages/man2/dup.2.html
+           *
+           * In-memory streams simply use a shared object to accomplish this, so that
+           * updating either property on one fd will change it on duplicated fds as well.
+           *
+           * Node, however, creates a node fd upon opening a file, and closing
+           * the node fd will also break duplicated fds. To work around this,
+           * we increment the refcount for the fd upon duplicating the stream, and
+           * only close the fd when all streams referencing the fd are closed.
+           */
+          refcount: {
+            get: function() { return this.shared.refcount; },
+            set: function(value) { this.shared.refcount = value; }
+	  }
+#endif
         });
       }
-      // clone it, so we can return an instance of FSStream
+
       var newStream = new FS.FSStream();
       for (var p in stream) {
-        newStream[p] = stream[p];
+        if (sharedProperties.indexOf(p) >= 0) {
+          if (!("shared" in newStream)) {
+            newStream.shared = {};
+          }
+          newStream.shared[p] = stream[p];
+        } else {
+          newStream[p] = stream[p];
+        }
+      }
+
+#if NODERAWFS
+      if ("refcount" in newStream.shared) {
+        newStream.refcount += 1;
       }
-      stream = newStream;
+#endif
+
+      return newStream;
+    },
+    // TODO parameterize this function such that a stream
+    // object isn't directly passed in. not possible until
+    // SOCKFS is completed.
+    createStream: function(stream, fd_start, fd_end) {
+      // clone it, so we can return an instance of FSStream
+      stream = FS.duplicateStream(stream);
       var fd = FS.nextfd(fd_start, fd_end);
       stream.fd = fd;
       FS.streams[fd] = stream;

src/library_noderawfs.js

@@ -51,17 +51,28 @@ mergeInto(LibraryManager.library, {
       if (typeof flags === "string") {
         flags = VFS.modeStringToFlags(flags)
       }
+
       var nfd = fs.openSync(path, NODEFS.flagsForNode(flags), mode);
       var fd = suggestFD != null ? suggestFD : FS.nextfd(nfd);
-      var stream = { fd: fd, nfd: nfd, position: 0, path: path, flags: flags, seekable: true };
-      FS.streams[fd] = stream;
+      var stream = FS.createStream({
+        fd: fd,
+        nfd: nfd,
+        refcount: 1,
+        position: 0,
+        path: path,
+        flags: flags,
+        seekable: true,
+      }, fd, fd);
       return stream;
     },
     close: function(stream) {
-      if (!stream.stream_ops) {
+      stream.shared.refcount -= 1;
+
+      if (!stream.stream_ops && !stream.shared.refcount) {
         // this stream is created by in-memory filesystem
         fs.closeSync(stream.nfd);
       }
+
       FS.closeStream(stream.fd);
     },
     llseek: function(stream, offset, whence) {

src/library_syscall.js

@@ -128,10 +128,15 @@ var SyscallsLibrary = {
       }
       return 0;
     },
-    doDup: function(path, flags, suggestFD) {
+    doDup: function(stream, suggestFD) {
       var suggest = FS.getStream(suggestFD);
       if (suggest) FS.close(suggest);
-      return FS.open(path, flags, 0, suggestFD, suggestFD).fd;
+
+      var newStream = FS.duplicateStream(stream);
+      newStream.fd = suggestFD;
+
+      FS.streams[suggestFD] = newStream;
+      return suggestFD;
     },
     doReadv: function(stream, iov, iovcnt, offset) {
       var ret = 0;
@@ -366,7 +371,7 @@ var SyscallsLibrary = {
   },
   __syscall41: function(which, varargs) { // dup
     var old = SYSCALLS.getStreamFromFD();
-    return FS.open(old.path, old.flags, 0).fd;
+    return SYSCALLS.doDup(old, FS.nextfd());
   },
   __syscall42__deps: ['$PIPEFS'],
   __syscall42: function(which, varargs) { // pipe
@@ -459,7 +464,7 @@ var SyscallsLibrary = {
   __syscall63: function(which, varargs) { // dup2
     var old = SYSCALLS.getStreamFromFD(), suggestFD = SYSCALLS.get();
     if (old.fd === suggestFD) return suggestFD;
-    return SYSCALLS.doDup(old.path, old.flags, suggestFD);
+    return SYSCALLS.doDup(old, suggestFD);
   },
   __syscall64__deps: ['$PROCINFO'],
   __syscall64: function(which, varargs) { // getppid
@@ -1074,14 +1079,14 @@ var SyscallsLibrary = {
 #else
     var stream = SYSCALLS.getStreamFromFD(), cmd = SYSCALLS.get();
     switch (cmd) {
+      // TODO: FreeBSD also defines F_DUP2FD and F_DUP2FD_CLOEXEC
+      case {{{ cDefine('F_DUPFD_CLOEXEC') }}}: // ignore O_CLOEXEC in single process environment
       case {{{ cDefine('F_DUPFD') }}}: {
         var arg = SYSCALLS.get();
         if (arg < 0) {
           return -{{{ cDefine('EINVAL') }}};
         }
-        var newStream;
-        newStream = FS.open(stream.path, stream.flags, 0, arg);
-        return newStream.fd;
+        return SYSCALLS.doDup(stream, FS.nextfd(arg));
       }
       case {{{ cDefine('F_GETFD') }}}:
       case {{{ cDefine('F_SETFD') }}}:
@@ -1308,8 +1313,13 @@ var SyscallsLibrary = {
 #if ASSERTIONS
     assert(!flags);
 #endif
-    if (old.fd === suggestFD) return -{{{ cDefine('EINVAL') }}};
-    return SYSCALLS.doDup(old.path, old.flags, suggestFD);
+    if (old.fd === suggestFD) return -ERRNO_CODES.EINVAL;
+
+    // intentionally ignore flags, since O_CLOEXEC make no sense in
+    // singleprocess environment
+    if (flags & ~{{{ cDefine('O_CLOEXEC') }}}) return -ERRNO_CODES.EINVAL;
+
+    return SYSCALLS.doDup(old, suggestFD);
   },
   __syscall331: function(which, varargs) { // pipe2
     return -{{{ cDefine('ENOSYS') }}}; // unsupported feature

src/struct_info.json

@@ -129,9 +129,11 @@
             "F_SETLK64", 
             "F_GETLK", 
             "S_ISVTX", 
+            "O_CLOEXEC",
             "O_RDONLY", 
             "O_ACCMODE", 
             "F_DUPFD", 
+            "F_DUPFD_CLOEXEC",
             "F_SETLK", 
             "O_WRONLY",
             "AT_FDCWD",

tests/unistd/dup.c

@@ -12,6 +12,11 @@
 
 int main() {
   int f, f2, f3;
+  off_t offset;
+
+  char *fnam = "/tmp/emscripten_test_file";
+  remove(fnam);
+  errno = 0;
 
   printf("DUP\n");
   f = open("/", O_RDONLY);
@@ -39,5 +44,52 @@ int main() {
   printf("\n");
   errno = 0;
 
+  f = open(fnam, O_RDWR | O_CREAT, S_IRUSR | S_IRGRP | S_IROTH);
+  close(f);
+
+  printf("DUP3\n");
+  f = open(fnam, O_RDONLY);
+  f2 = open(fnam, O_RDONLY);
+  f3 = dup(f);
+  printf("errno: %d\n", errno);
+  printf("f: %d\n", f != f2 && f != f3);
+  printf("f2,f3: %d\n", f2 != f3);
+
+  // dup()ed file descriptors should share all flags and even seek position
+  offset = lseek(f3, 0, SEEK_CUR);
+  printf("1. f3 offset was %d.    Should be 0\n", (int)offset);
+  offset = lseek(f, 1, SEEK_SET);
+  printf("2. f  offset set to %d. Should be 1\n", (int)offset);
+  offset = lseek(f2, 2, SEEK_SET);
+  printf("3. f2 offset set to %d. Should be 2\n", (int)offset);
+  offset = lseek(f, 0, SEEK_CUR);
+  printf("4. f  offset now is %d. Should be 1\n", (int)offset);
+  offset = lseek(f2, 0, SEEK_CUR);
+  printf("5. f2 offset now is %d. Should be 2\n", (int)offset);
+  offset = lseek(f3, 0, SEEK_CUR);
+  printf("6. f3 offset now is %d. Should be 1 (and not 0)\n", (int)offset);
+  offset = lseek(f3, 3, SEEK_SET);
+  printf("7. f3 offset set to %d. Should be 3\n", (int)offset);
+  offset = lseek(f, 0, SEEK_CUR);
+  printf("8. f  offset now is %d. Should be 3 (and not 1)\n", (int)offset);
+
+  printf("close(f1): %d\n", close(f));
+  printf("close(f2): %d\n", close(f2));
+  printf("close(f3): %d\n", close(f3));
+  printf("\n");
+  errno = 0;
+
+  printf("DUP4\n");
+  f = open("/", O_RDONLY);
+  f2 = open("/", O_RDONLY);
+  f3 = dup2(f, f2);
+  printf("errno: %d\n", errno);
+  printf("f: %d\n", f != f2 && f != f3);
+  printf("f2,f3: %d\n", f2 == f3);
+  printf("close(f1): %d\n", close(f));
+  printf("close(f2): %d\n", close(f2));
+  printf("close(f3): %d\n", close(f3));
+  errno = 0;
+
   return 0;
 }

tests/unistd/dup.out

@@ -13,3 +13,27 @@ f2,f3: 1
 close(f1): 0
 close(f2): 0
 close(f3): -1
+
+DUP3
+errno: 0
+f: 1
+f2,f3: 1
+1. f3 offset was 0.    Should be 0
+2. f  offset set to 1. Should be 1
+3. f2 offset set to 2. Should be 2
+4. f  offset now is 1. Should be 1
+5. f2 offset now is 2. Should be 2
+6. f3 offset now is 1. Should be 1 (and not 0)
+7. f3 offset set to 3. Should be 3
+8. f  offset now is 3. Should be 3 (and not 1)
+close(f1): 0
+close(f2): 0
+close(f3): 0
+
+DUP4
+errno: 0
+f: 1
+f2,f3: 1
+close(f1): 0
+close(f2): 0
+close(f3): -1