From b278cbb6e88f796db5e3e043744bdf28639c0b1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=9A=D0=BE=D1=80=D0=B5=D0=BD=D0=B1=D0=B5=D1=80=D0=B3=20?=
=?UTF-8?q?=D0=9C=D0=B0=D1=80=D0=BA=20=28imac=29?= <socketpair@gmail.com>
Date: Thu, 7 Jan 2016 04:23:09 +0500
Subject: [PATCH] Fix dup*() syscalls
Rework dup syscalls so file descriptors share all flags and also seek position.
---
src/library_fs.js | 58 +++++++++++++++++++++++++++++++++++-----
src/library_noderawfs.js | 17 +++++++++---
src/library_syscall.js | 28 ++++++++++++-------
src/struct_info.json | 2 ++
tests/unistd/dup.c | 52 +++++++++++++++++++++++++++++++++++
tests/unistd/dup.out | 24 +++++++++++++++++
6 files changed, 162 insertions(+), 19 deletions(-)
diff --git a/src/library_fs.js b/src/library_fs.js
index 2f7b0be52a7d2..f7bf42444a050 100644
--- a/src/library_fs.js
+++ b/src/library_fs.js
@@ -400,10 +400,8 @@ 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
if (!FS.FSStream) {
FS.FSStream = function(){};
FS.FSStream.prototype = {};
@@ -421,15 +419,61 @@ 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();
+ if (!newStream.shared) {
+ newStream.shared = {};
+ }
+
for (var p in stream) {
newStream[p] = stream[p];
}
- stream = newStream;
+
+#if NODERAWFS
+ if (newStream.shared.refcount) {
+ newStream.refcount += 1;
+ }
+#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;
diff --git a/src/library_noderawfs.js b/src/library_noderawfs.js
index a3f65cc5729ee..0f8b5748b1d4a 100644
--- a/src/library_noderawfs.js
+++ b/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) {
diff --git a/src/library_syscall.js b/src/library_syscall.js
index 78f5dd8d60cd3..49383168e4869 100644
--- a/src/library_syscall.js
+++ b/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
@@ -1076,14 +1081,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') }}}:
@@ -1310,8 +1315,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
diff --git a/src/struct_info.json b/src/struct_info.json
index ae99084210a77..1471c7c947031 100644
--- a/src/struct_info.json
+++ b/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",
diff --git a/tests/unistd/dup.c b/tests/unistd/dup.c
index 99d339e001aac..785222b11ac86 100644
--- a/tests/unistd/dup.c
+++ b/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;
}
diff --git a/tests/unistd/dup.out b/tests/unistd/dup.out
index 0b258ba3c7edc..56096bb310c86 100644
--- a/tests/unistd/dup.out
+++ b/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