Skip to content

Commit b278cbb

Browse files
socketpairjakogut
authored andcommittedDec 18, 2019
Fix dup*() syscalls
Rework dup syscalls so file descriptors share all flags and also seek position.
1 parent 748c80c commit b278cbb

File tree

6 files changed

+162
-19
lines changed

6 files changed

+162
-19
lines changed
 

‎src/library_fs.js

+51-7
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,8 @@ mergeInto(LibraryManager.library, {
400400
getStream: function(fd) {
401401
return FS.streams[fd];
402402
},
403-
// TODO parameterize this function such that a stream
404-
// object isn't directly passed in. not possible until
405-
// SOCKFS is completed.
406-
createStream: function(stream, fd_start, fd_end) {
403+
duplicateStream: function(stream) {
404+
// Some properties are shared between duplicates
407405
if (!FS.FSStream) {
408406
FS.FSStream = function(){};
409407
FS.FSStream.prototype = {};
@@ -421,15 +419,61 @@ mergeInto(LibraryManager.library, {
421419
},
422420
isAppend: {
423421
get: function() { return (this.flags & {{{ cDefine('O_APPEND') }}}); }
424-
}
422+
},
423+
flags: {
424+
get: function() { return this.shared.flags; },
425+
set: function(value) { this.shared.flags = value; }
426+
},
427+
position: {
428+
get: function() { return this.shared.position; },
429+
set: function(value) { this.shared.position = value; }
430+
},
431+
#if NODERAWFS
432+
/* refcount is a stream attribute only used by NODERAWFS
433+
*
434+
* Duplicated streams share flags and seek position, as described in dup(2):
435+
*
436+
* http://man7.org/linux/man-pages/man2/dup.2.html
437+
*
438+
* In-memory streams simply use a shared object to accomplish this, so that
439+
* updating either property on one fd will change it on duplicated fds as well.
440+
*
441+
* Node, however, creates a node fd upon opening a file, and closing
442+
* the node fd will also break duplicated fds. To work around this,
443+
* we increment the refcount for the fd upon duplicating the stream, and
444+
* only close the fd when all streams referencing the fd are closed.
445+
*/
446+
refcount: {
447+
get: function() { return this.shared.refcount; },
448+
set: function(value) { this.shared.refcount = value; }
449+
}
450+
#endif
425451
});
426452
}
427-
// clone it, so we can return an instance of FSStream
453+
428454
var newStream = new FS.FSStream();
455+
if (!newStream.shared) {
456+
newStream.shared = {};
457+
}
458+
429459
for (var p in stream) {
430460
newStream[p] = stream[p];
431461
}
432-
stream = newStream;
462+
463+
#if NODERAWFS
464+
if (newStream.shared.refcount) {
465+
newStream.refcount += 1;
466+
}
467+
#endif
468+
469+
return newStream;
470+
},
471+
// TODO parameterize this function such that a stream
472+
// object isn't directly passed in. not possible until
473+
// SOCKFS is completed.
474+
createStream: function(stream, fd_start, fd_end) {
475+
// clone it, so we can return an instance of FSStream
476+
stream = FS.duplicateStream(stream);
433477
var fd = FS.nextfd(fd_start, fd_end);
434478
stream.fd = fd;
435479
FS.streams[fd] = stream;

‎src/library_noderawfs.js

+14-3
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,28 @@ mergeInto(LibraryManager.library, {
5151
if (typeof flags === "string") {
5252
flags = VFS.modeStringToFlags(flags)
5353
}
54+
5455
var nfd = fs.openSync(path, NODEFS.flagsForNode(flags), mode);
5556
var fd = suggestFD != null ? suggestFD : FS.nextfd(nfd);
56-
var stream = { fd: fd, nfd: nfd, position: 0, path: path, flags: flags, seekable: true };
57-
FS.streams[fd] = stream;
57+
var stream = FS.createStream({
58+
fd: fd,
59+
nfd: nfd,
60+
refcount: 1,
61+
position: 0,
62+
path: path,
63+
flags: flags,
64+
seekable: true,
65+
}, fd, fd);
5866
return stream;
5967
},
6068
close: function(stream) {
61-
if (!stream.stream_ops) {
69+
stream.shared.refcount -= 1;
70+
71+
if (!stream.stream_ops && !stream.shared.refcount) {
6272
// this stream is created by in-memory filesystem
6373
fs.closeSync(stream.nfd);
6474
}
75+
6576
FS.closeStream(stream.fd);
6677
},
6778
llseek: function(stream, offset, whence) {

‎src/library_syscall.js

+19-9
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,15 @@ var SyscallsLibrary = {
128128
}
129129
return 0;
130130
},
131-
doDup: function(path, flags, suggestFD) {
131+
doDup: function(stream, suggestFD) {
132132
var suggest = FS.getStream(suggestFD);
133133
if (suggest) FS.close(suggest);
134-
return FS.open(path, flags, 0, suggestFD, suggestFD).fd;
134+
135+
var newStream = FS.duplicateStream(stream);
136+
newStream.fd = suggestFD;
137+
138+
FS.streams[suggestFD] = newStream;
139+
return suggestFD;
135140
},
136141
doReadv: function(stream, iov, iovcnt, offset) {
137142
var ret = 0;
@@ -366,7 +371,7 @@ var SyscallsLibrary = {
366371
},
367372
__syscall41: function(which, varargs) { // dup
368373
var old = SYSCALLS.getStreamFromFD();
369-
return FS.open(old.path, old.flags, 0).fd;
374+
return SYSCALLS.doDup(old, FS.nextfd());
370375
},
371376
__syscall42__deps: ['$PIPEFS'],
372377
__syscall42: function(which, varargs) { // pipe
@@ -459,7 +464,7 @@ var SyscallsLibrary = {
459464
__syscall63: function(which, varargs) { // dup2
460465
var old = SYSCALLS.getStreamFromFD(), suggestFD = SYSCALLS.get();
461466
if (old.fd === suggestFD) return suggestFD;
462-
return SYSCALLS.doDup(old.path, old.flags, suggestFD);
467+
return SYSCALLS.doDup(old, suggestFD);
463468
},
464469
__syscall64__deps: ['$PROCINFO'],
465470
__syscall64: function(which, varargs) { // getppid
@@ -1076,14 +1081,14 @@ var SyscallsLibrary = {
10761081
#else
10771082
var stream = SYSCALLS.getStreamFromFD(), cmd = SYSCALLS.get();
10781083
switch (cmd) {
1084+
// TODO: FreeBSD also defines F_DUP2FD and F_DUP2FD_CLOEXEC
1085+
case {{{ cDefine('F_DUPFD_CLOEXEC') }}}: // ignore O_CLOEXEC in single process environment
10791086
case {{{ cDefine('F_DUPFD') }}}: {
10801087
var arg = SYSCALLS.get();
10811088
if (arg < 0) {
10821089
return -{{{ cDefine('EINVAL') }}};
10831090
}
1084-
var newStream;
1085-
newStream = FS.open(stream.path, stream.flags, 0, arg);
1086-
return newStream.fd;
1091+
return SYSCALLS.doDup(stream, FS.nextfd(arg));
10871092
}
10881093
case {{{ cDefine('F_GETFD') }}}:
10891094
case {{{ cDefine('F_SETFD') }}}:
@@ -1310,8 +1315,13 @@ var SyscallsLibrary = {
13101315
#if ASSERTIONS
13111316
assert(!flags);
13121317
#endif
1313-
if (old.fd === suggestFD) return -{{{ cDefine('EINVAL') }}};
1314-
return SYSCALLS.doDup(old.path, old.flags, suggestFD);
1318+
if (old.fd === suggestFD) return -ERRNO_CODES.EINVAL;
1319+
1320+
// intentionally ignore flags, since O_CLOEXEC make no sense in
1321+
// singleprocess environment
1322+
if (flags & ~{{{ cDefine('O_CLOEXEC') }}}) return -ERRNO_CODES.EINVAL;
1323+
1324+
return SYSCALLS.doDup(old, suggestFD);
13151325
},
13161326
__syscall331: function(which, varargs) { // pipe2
13171327
return -{{{ cDefine('ENOSYS') }}}; // unsupported feature

‎src/struct_info.json

+2
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,11 @@
129129
"F_SETLK64",
130130
"F_GETLK",
131131
"S_ISVTX",
132+
"O_CLOEXEC",
132133
"O_RDONLY",
133134
"O_ACCMODE",
134135
"F_DUPFD",
136+
"F_DUPFD_CLOEXEC",
135137
"F_SETLK",
136138
"O_WRONLY",
137139
"AT_FDCWD",

‎tests/unistd/dup.c

+52
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212

1313
int main() {
1414
int f, f2, f3;
15+
off_t offset;
16+
17+
char *fnam = "/tmp/emscripten_test_file";
18+
remove(fnam);
19+
errno = 0;
1520

1621
printf("DUP\n");
1722
f = open("/", O_RDONLY);
@@ -39,5 +44,52 @@ int main() {
3944
printf("\n");
4045
errno = 0;
4146

47+
f = open(fnam, O_RDWR | O_CREAT, S_IRUSR | S_IRGRP | S_IROTH);
48+
close(f);
49+
50+
printf("DUP3\n");
51+
f = open(fnam, O_RDONLY);
52+
f2 = open(fnam, O_RDONLY);
53+
f3 = dup(f);
54+
printf("errno: %d\n", errno);
55+
printf("f: %d\n", f != f2 && f != f3);
56+
printf("f2,f3: %d\n", f2 != f3);
57+
58+
// dup()ed file descriptors should share all flags and even seek position
59+
offset = lseek(f3, 0, SEEK_CUR);
60+
printf("1. f3 offset was %d. Should be 0\n", (int)offset);
61+
offset = lseek(f, 1, SEEK_SET);
62+
printf("2. f offset set to %d. Should be 1\n", (int)offset);
63+
offset = lseek(f2, 2, SEEK_SET);
64+
printf("3. f2 offset set to %d. Should be 2\n", (int)offset);
65+
offset = lseek(f, 0, SEEK_CUR);
66+
printf("4. f offset now is %d. Should be 1\n", (int)offset);
67+
offset = lseek(f2, 0, SEEK_CUR);
68+
printf("5. f2 offset now is %d. Should be 2\n", (int)offset);
69+
offset = lseek(f3, 0, SEEK_CUR);
70+
printf("6. f3 offset now is %d. Should be 1 (and not 0)\n", (int)offset);
71+
offset = lseek(f3, 3, SEEK_SET);
72+
printf("7. f3 offset set to %d. Should be 3\n", (int)offset);
73+
offset = lseek(f, 0, SEEK_CUR);
74+
printf("8. f offset now is %d. Should be 3 (and not 1)\n", (int)offset);
75+
76+
printf("close(f1): %d\n", close(f));
77+
printf("close(f2): %d\n", close(f2));
78+
printf("close(f3): %d\n", close(f3));
79+
printf("\n");
80+
errno = 0;
81+
82+
printf("DUP4\n");
83+
f = open("/", O_RDONLY);
84+
f2 = open("/", O_RDONLY);
85+
f3 = dup2(f, f2);
86+
printf("errno: %d\n", errno);
87+
printf("f: %d\n", f != f2 && f != f3);
88+
printf("f2,f3: %d\n", f2 == f3);
89+
printf("close(f1): %d\n", close(f));
90+
printf("close(f2): %d\n", close(f2));
91+
printf("close(f3): %d\n", close(f3));
92+
errno = 0;
93+
4294
return 0;
4395
}

‎tests/unistd/dup.out

+24
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,27 @@ f2,f3: 1
1313
close(f1): 0
1414
close(f2): 0
1515
close(f3): -1
16+
17+
DUP3
18+
errno: 0
19+
f: 1
20+
f2,f3: 1
21+
1. f3 offset was 0. Should be 0
22+
2. f offset set to 1. Should be 1
23+
3. f2 offset set to 2. Should be 2
24+
4. f offset now is 1. Should be 1
25+
5. f2 offset now is 2. Should be 2
26+
6. f3 offset now is 1. Should be 1 (and not 0)
27+
7. f3 offset set to 3. Should be 3
28+
8. f offset now is 3. Should be 3 (and not 1)
29+
close(f1): 0
30+
close(f2): 0
31+
close(f3): 0
32+
33+
DUP4
34+
errno: 0
35+
f: 1
36+
f2,f3: 1
37+
close(f1): 0
38+
close(f2): 0
39+
close(f3): -1

0 commit comments

Comments
 (0)
Failed to load comments.