From 1e52bb68a9d202eb70095d04b1883b4bd3a521b2 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 22 Feb 2024 11:01:30 -0800 Subject: [PATCH 1/2] Fix dup in nodefs --- src/library_fs.js | 5 +++++ src/library_nodefs.js | 6 +++++- src/library_syscall.js | 6 +++--- test/fs/test_nodefs_dup.c | 39 +++++++++++++++++++++++++++++++++++++++ test/test_core.py | 10 ++++++++++ 5 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 test/fs/test_nodefs_dup.c diff --git a/src/library_fs.js b/src/library_fs.js index b6f0a5092b23d..09ce3763d87f3 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -471,6 +471,11 @@ FS.staticInit();` + closeStream(fd) { FS.streams[fd] = null; }, + dupStream(origStream, fd = -1) { + var stream = FS.createStream(origStream, fd); + stream.stream_ops?.dup?.(stream); + return stream; + }, // // devices diff --git a/src/library_nodefs.js b/src/library_nodefs.js index 81864ffcc956f..90efdf20b82c0 100644 --- a/src/library_nodefs.js +++ b/src/library_nodefs.js @@ -251,6 +251,7 @@ addToLibrary({ var path = NODEFS.realPath(stream.node); try { if (FS.isFile(stream.node.mode)) { + stream.shared.refcount = 1; stream.nfd = fs.openSync(path, NODEFS.flagsForNode(stream.flags)); } } catch (e) { @@ -260,7 +261,7 @@ addToLibrary({ }, close(stream) { try { - if (FS.isFile(stream.node.mode) && stream.nfd) { + if (FS.isFile(stream.node.mode) && stream.nfd && --stream.shared.refcount === 0) { fs.closeSync(stream.nfd); } } catch (e) { @@ -268,6 +269,9 @@ addToLibrary({ throw new FS.ErrnoError(NODEFS.convertNodeCode(e)); } }, + dup(stream) { + stream.shared.refcount ++; + }, read(stream, buffer, offset, length, position) { // Node.js < 6 compatibility: node errors on 0 length reads if (length === 0) return 0; diff --git a/src/library_syscall.js b/src/library_syscall.js index b078bd71cf7c4..69ea6f8b210b1 100644 --- a/src/library_syscall.js +++ b/src/library_syscall.js @@ -183,7 +183,7 @@ var SyscallsLibrary = { }, __syscall_dup: (fd) => { var old = SYSCALLS.getStreamFromFD(fd); - return FS.createStream(old).fd; + return FS.dupStream(old).fd; }, __syscall_pipe__deps: ['$PIPEFS'], __syscall_pipe: (fdPtr) => { @@ -760,7 +760,7 @@ var SyscallsLibrary = { arg++; } var newStream; - newStream = FS.createStream(stream, arg); + newStream = FS.dupStream(stream, arg); return newStream.fd; } case {{{ cDefs.F_GETFD }}}: @@ -1007,7 +1007,7 @@ var SyscallsLibrary = { if (old.fd === newfd) return -{{{ cDefs.EINVAL }}}; var existing = FS.getStream(newfd); if (existing) FS.close(existing); - return FS.createStream(old, newfd).fd; + return FS.dupStream(old, newfd).fd; }, }; diff --git a/test/fs/test_nodefs_dup.c b/test/fs/test_nodefs_dup.c new file mode 100644 index 0000000000000..028b994b7b771 --- /dev/null +++ b/test/fs/test_nodefs_dup.c @@ -0,0 +1,39 @@ +/* + * Copyright 2018 The Emscripten Authors. All rights reserved. + * Emscripten is available under two separate licenses, the MIT license and the + * University of Illinois/NCSA Open Source License. Both these licenses can be + * found in the LICENSE file. + */ + +#include +#include +#include +#include + +#ifdef NODERAWFS +#define CWD "" +#else +#define CWD "/working/" +#endif + +int main(void) +{ + EM_ASM( +#ifdef NODERAWFS + FS.close(FS.open('test.txt', 'w')); +#else + FS.mkdir('/working'); + FS.mount(NODEFS, {root: '.'}, '/working'); + FS.close(FS.open('/working/test.txt', 'w')); +#endif + ); + int fd1 = open(CWD "test.txt", O_RDONLY); + int fd2 = dup(fd1); + int fd3 = fcntl(fd1, F_DUPFD_CLOEXEC, 0); + + printf("fd1: %d, fd2: %d\n", fd1, fd2); + printf("close(fd1): %d\n", close(fd1)); + printf("close(fd2): %d\n", close(fd2)); + printf("close(fd3): %d\n", close(fd3)); + return 0; +} diff --git a/test/test_core.py b/test/test_core.py index 1dd31b8966199..e00575b4d4cc2 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5749,6 +5749,16 @@ def test_fs_nodefs_cloexec(self): self.emcc_args += ['-lnodefs.js'] self.do_runf('fs/test_nodefs_cloexec.c', 'success') + @also_with_noderawfs + @requires_node + def test_fs_nodefs_dup(self): + if self.get_setting('WASMFS'): + self.set_setting('FORCE_FILESYSTEM') + self.emcc_args += ['-lnodefs.js'] + expected = "fd1: 3, fd2: 4\nclose(fd1): 0\nclose(fd2): 0\nclose(fd3): 0" + + self.do_runf('fs/test_nodefs_dup.c', expected) + @requires_node def test_fs_nodefs_home(self): self.set_setting('FORCE_FILESYSTEM') From 1f142986493578f7a41e513e0d83ae2fc03275de Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 22 Feb 2024 19:00:11 -0800 Subject: [PATCH 2/2] Rearrange test a bit --- src/library_nodefs.js | 2 +- test/fs/test_nodefs_dup.c | 20 +++++++++++++------- test/test_core.py | 4 +--- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/library_nodefs.js b/src/library_nodefs.js index 90efdf20b82c0..ace50458c9c43 100644 --- a/src/library_nodefs.js +++ b/src/library_nodefs.js @@ -270,7 +270,7 @@ addToLibrary({ } }, dup(stream) { - stream.shared.refcount ++; + stream.shared.refcount++; }, read(stream, buffer, offset, length, position) { // Node.js < 6 compatibility: node errors on 0 length reads diff --git a/test/fs/test_nodefs_dup.c b/test/fs/test_nodefs_dup.c index 028b994b7b771..abf34935b11c3 100644 --- a/test/fs/test_nodefs_dup.c +++ b/test/fs/test_nodefs_dup.c @@ -5,9 +5,10 @@ * found in the LICENSE file. */ -#include -#include +#include #include +#include +#include #include #ifdef NODERAWFS @@ -27,13 +28,18 @@ int main(void) FS.close(FS.open('/working/test.txt', 'w')); #endif ); - int fd1 = open(CWD "test.txt", O_RDONLY); + int fd1 = open(CWD "test.txt", O_WRONLY); int fd2 = dup(fd1); int fd3 = fcntl(fd1, F_DUPFD_CLOEXEC, 0); - printf("fd1: %d, fd2: %d\n", fd1, fd2); - printf("close(fd1): %d\n", close(fd1)); - printf("close(fd2): %d\n", close(fd2)); - printf("close(fd3): %d\n", close(fd3)); + assert(fd1 == 3); + assert(fd2 == 4); + assert(fd3 == 5); + assert(close(fd1) == 0); + assert(write(fd2, "abcdef", 6) == 6); + assert(close(fd2) == 0); + assert(write(fd3, "ghijkl", 6) == 6); + assert(close(fd3) == 0); + printf("success\n"); return 0; } diff --git a/test/test_core.py b/test/test_core.py index e00575b4d4cc2..8b67c80ed1a05 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5755,9 +5755,7 @@ def test_fs_nodefs_dup(self): if self.get_setting('WASMFS'): self.set_setting('FORCE_FILESYSTEM') self.emcc_args += ['-lnodefs.js'] - expected = "fd1: 3, fd2: 4\nclose(fd1): 0\nclose(fd2): 0\nclose(fd3): 0" - - self.do_runf('fs/test_nodefs_dup.c', expected) + self.do_runf('fs/test_nodefs_dup.c', 'success') @requires_node def test_fs_nodefs_home(self):