|
| 1 | +From 135baa3d4cc211c0145d2e1df116e39f60df4f91 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Hood Chatham < [email protected]> |
| 3 | +Date: Thu, 22 Feb 2024 21:47:13 -0800 |
| 4 | +Subject: [PATCH] Fix dup in nodefs by refcounting nodefs file descriptors |
| 5 | + (#21399) |
| 6 | + |
| 7 | +--- |
| 8 | + src/library_fs.js | 5 +++++ |
| 9 | + src/library_nodefs.js | 6 +++++- |
| 10 | + src/library_syscall.js | 6 +++--- |
| 11 | + test/fs/test_nodefs_dup.c | 45 +++++++++++++++++++++++++++++++++++++++ |
| 12 | + test/test_core.py | 8 +++++++ |
| 13 | + 5 files changed, 66 insertions(+), 4 deletions(-) |
| 14 | + create mode 100644 test/fs/test_nodefs_dup.c |
| 15 | + |
| 16 | +diff --git a/src/library_fs.js b/src/library_fs.js |
| 17 | +index f5d16b86c..379e65268 100644 |
| 18 | +--- a/src/library_fs.js |
| 19 | ++++ b/src/library_fs.js |
| 20 | +@@ -471,6 +471,11 @@ FS.staticInit();` + |
| 21 | + closeStream(fd) { |
| 22 | + FS.streams[fd] = null; |
| 23 | + }, |
| 24 | ++ dupStream(origStream, fd = -1) { |
| 25 | ++ var stream = FS.createStream(origStream, fd); |
| 26 | ++ stream.stream_ops?.dup?.(stream); |
| 27 | ++ return stream; |
| 28 | ++ }, |
| 29 | + |
| 30 | + // |
| 31 | + // devices |
| 32 | +diff --git a/src/library_nodefs.js b/src/library_nodefs.js |
| 33 | +index 81864ffcc..ace50458c 100644 |
| 34 | +--- a/src/library_nodefs.js |
| 35 | ++++ b/src/library_nodefs.js |
| 36 | +@@ -251,6 +251,7 @@ addToLibrary({ |
| 37 | + var path = NODEFS.realPath(stream.node); |
| 38 | + try { |
| 39 | + if (FS.isFile(stream.node.mode)) { |
| 40 | ++ stream.shared.refcount = 1; |
| 41 | + stream.nfd = fs.openSync(path, NODEFS.flagsForNode(stream.flags)); |
| 42 | + } |
| 43 | + } catch (e) { |
| 44 | +@@ -260,7 +261,7 @@ addToLibrary({ |
| 45 | + }, |
| 46 | + close(stream) { |
| 47 | + try { |
| 48 | +- if (FS.isFile(stream.node.mode) && stream.nfd) { |
| 49 | ++ if (FS.isFile(stream.node.mode) && stream.nfd && --stream.shared.refcount === 0) { |
| 50 | + fs.closeSync(stream.nfd); |
| 51 | + } |
| 52 | + } catch (e) { |
| 53 | +@@ -268,6 +269,9 @@ addToLibrary({ |
| 54 | + throw new FS.ErrnoError(NODEFS.convertNodeCode(e)); |
| 55 | + } |
| 56 | + }, |
| 57 | ++ dup(stream) { |
| 58 | ++ stream.shared.refcount++; |
| 59 | ++ }, |
| 60 | + read(stream, buffer, offset, length, position) { |
| 61 | + // Node.js < 6 compatibility: node errors on 0 length reads |
| 62 | + if (length === 0) return 0; |
| 63 | +diff --git a/src/library_syscall.js b/src/library_syscall.js |
| 64 | +index b078bd71c..69ea6f8b2 100644 |
| 65 | +--- a/src/library_syscall.js |
| 66 | ++++ b/src/library_syscall.js |
| 67 | +@@ -183,7 +183,7 @@ var SyscallsLibrary = { |
| 68 | + }, |
| 69 | + __syscall_dup: (fd) => { |
| 70 | + var old = SYSCALLS.getStreamFromFD(fd); |
| 71 | +- return FS.createStream(old).fd; |
| 72 | ++ return FS.dupStream(old).fd; |
| 73 | + }, |
| 74 | + __syscall_pipe__deps: ['$PIPEFS'], |
| 75 | + __syscall_pipe: (fdPtr) => { |
| 76 | +@@ -760,7 +760,7 @@ var SyscallsLibrary = { |
| 77 | + arg++; |
| 78 | + } |
| 79 | + var newStream; |
| 80 | +- newStream = FS.createStream(stream, arg); |
| 81 | ++ newStream = FS.dupStream(stream, arg); |
| 82 | + return newStream.fd; |
| 83 | + } |
| 84 | + case {{{ cDefs.F_GETFD }}}: |
| 85 | +@@ -1007,7 +1007,7 @@ var SyscallsLibrary = { |
| 86 | + if (old.fd === newfd) return -{{{ cDefs.EINVAL }}}; |
| 87 | + var existing = FS.getStream(newfd); |
| 88 | + if (existing) FS.close(existing); |
| 89 | +- return FS.createStream(old, newfd).fd; |
| 90 | ++ return FS.dupStream(old, newfd).fd; |
| 91 | + }, |
| 92 | + }; |
| 93 | + |
| 94 | +diff --git a/test/fs/test_nodefs_dup.c b/test/fs/test_nodefs_dup.c |
| 95 | +new file mode 100644 |
| 96 | +index 000000000..abf34935b |
| 97 | +--- /dev/null |
| 98 | ++++ b/test/fs/test_nodefs_dup.c |
| 99 | +@@ -0,0 +1,45 @@ |
| 100 | ++/* |
| 101 | ++ * Copyright 2018 The Emscripten Authors. All rights reserved. |
| 102 | ++ * Emscripten is available under two separate licenses, the MIT license and the |
| 103 | ++ * University of Illinois/NCSA Open Source License. Both these licenses can be |
| 104 | ++ * found in the LICENSE file. |
| 105 | ++ */ |
| 106 | ++ |
| 107 | ++#include <assert.h> |
| 108 | ++#include <emscripten.h> |
| 109 | ++#include <fcntl.h> |
| 110 | ++#include <stdio.h> |
| 111 | ++#include <unistd.h> |
| 112 | ++ |
| 113 | ++#ifdef NODERAWFS |
| 114 | ++#define CWD "" |
| 115 | ++#else |
| 116 | ++#define CWD "/working/" |
| 117 | ++#endif |
| 118 | ++ |
| 119 | ++int main(void) |
| 120 | ++{ |
| 121 | ++ EM_ASM( |
| 122 | ++#ifdef NODERAWFS |
| 123 | ++ FS.close(FS.open('test.txt', 'w')); |
| 124 | ++#else |
| 125 | ++ FS.mkdir('/working'); |
| 126 | ++ FS.mount(NODEFS, {root: '.'}, '/working'); |
| 127 | ++ FS.close(FS.open('/working/test.txt', 'w')); |
| 128 | ++#endif |
| 129 | ++ ); |
| 130 | ++ int fd1 = open(CWD "test.txt", O_WRONLY); |
| 131 | ++ int fd2 = dup(fd1); |
| 132 | ++ int fd3 = fcntl(fd1, F_DUPFD_CLOEXEC, 0); |
| 133 | ++ |
| 134 | ++ assert(fd1 == 3); |
| 135 | ++ assert(fd2 == 4); |
| 136 | ++ assert(fd3 == 5); |
| 137 | ++ assert(close(fd1) == 0); |
| 138 | ++ assert(write(fd2, "abcdef", 6) == 6); |
| 139 | ++ assert(close(fd2) == 0); |
| 140 | ++ assert(write(fd3, "ghijkl", 6) == 6); |
| 141 | ++ assert(close(fd3) == 0); |
| 142 | ++ printf("success\n"); |
| 143 | ++ return 0; |
| 144 | ++} |
| 145 | +diff --git a/test/test_core.py b/test/test_core.py |
| 146 | +index 3f21eb5ef..f304f1366 100644 |
| 147 | +--- a/test/test_core.py |
| 148 | ++++ b/test/test_core.py |
| 149 | +@@ -5745,6 +5745,14 @@ Module = { |
| 150 | + self.emcc_args += ['-lnodefs.js'] |
| 151 | + self.do_runf('fs/test_nodefs_cloexec.c', 'success') |
| 152 | + |
| 153 | ++ @also_with_noderawfs |
| 154 | ++ @requires_node |
| 155 | ++ def test_fs_nodefs_dup(self): |
| 156 | ++ if self.get_setting('WASMFS'): |
| 157 | ++ self.set_setting('FORCE_FILESYSTEM') |
| 158 | ++ self.emcc_args += ['-lnodefs.js'] |
| 159 | ++ self.do_runf('fs/test_nodefs_dup.c', 'success') |
| 160 | ++ |
| 161 | + @requires_node |
| 162 | + def test_fs_nodefs_home(self): |
| 163 | + self.set_setting('FORCE_FILESYSTEM') |
| 164 | +-- |
| 165 | +2.34.1 |
| 166 | + |
0 commit comments