From 944a511127bd2fd3bc463712054cbe53d73c02c5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 18 Jan 2018 16:38:31 -0800 Subject: [PATCH 1/3] src: FatalException if env->SetImmediate callbacks throw Currently, if any env->SetImmediate callback uses any of the env->Throw{Whatever} variants, the exception is lost and nothing happens. This commit catches those with a v8::TryCatch and does a FatalException to tear down the process. --- src/env.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/env.cc b/src/env.cc index 706af7745416e0..04cbbb1c75b8d2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -280,7 +280,11 @@ void Environment::RunAndClearNativeImmediates() { std::vector list; native_immediate_callbacks_.swap(list); for (const auto& cb : list) { + v8::TryCatch try_catch(isolate()); cb.cb_(this, cb.data_); + if (try_catch.HasCaught()) { + FatalException(isolate(), try_catch); + } if (cb.keep_alive_) cb.keep_alive_->Reset(); if (cb.refed_) From 019d7fd84850751916d6406faffbd11ca958c765 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 11 Jan 2018 13:38:54 -0800 Subject: [PATCH 2/3] fs: add FD object to manage file descriptors In preparation for providinng a promisified fs API, introduce a FD wrapper class that automatically closes the file descriptor on garbage collection, helping to ensure that the fd will not leak. The object should still be explicitly closed, and a process warning will be emitted if it is closed on gc (a fatal exception will occur if closing during gc fails). The promisified fs API will use these objects instead of the numeric file descriptors in order to ensure that fd's are closed appropriately following promise resolve or reject operations. --- doc/api/fs.md | 149 ++++++++++++++ lib/fs.js | 30 +++ src/async_wrap.h | 1 + src/env.h | 1 + src/node_file.cc | 190 ++++++++++++++++++ src/node_file.h | 61 ++++++ test/parallel/test-fs-fd.js | 41 ++++ test/parallel/test-fs-open.js | 66 ++++-- test/sequential/test-async-wrap-getasyncid.js | 6 + 9 files changed, 531 insertions(+), 14 deletions(-) create mode 100644 test/parallel/test-fs-fd.js diff --git a/doc/api/fs.md b/doc/api/fs.md index f2430b0da09a78..963d62c7a3f244 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -291,6 +291,44 @@ explicitly synchronous use libuv's threadpool, which can have surprising and negative performance implications for some applications, see the [`UV_THREADPOOL_SIZE`][] documentation for more information. +## class: fs.FD + + +An `fs.FD` object is a wrapper for a numeric file descriptor. Instances of +`fs.FD` are distinct from numeric file descriptors in that, if the `fs.FD` is +not explicitly closed using the `fd.close()` method, they will automatically +close the file descriptor and will emit a process warning, thereby helping to +prevent memory leaks. + +Instances of the `fs.FD` object are created internally by the `fs.openFD()` +and `fs.openFDSync()` methods. + +### fd.close() + + +* Returns: {Promise} A `Promise` that will be resolved once the underlying + file descriptor is closed, or will reject if an error occurs while closing. + +Closes the file descriptor. + +```js +const fd = fs.openFDSync('thefile.txt', 'r'); +fd.close() + .then(() => console.log('ok')) + .catch(() => console.log('not ok')); +``` + +### fd.fd + + +Value: {number} The numeric file descriptor managed by the `FD` object. + ## Class: fs.FSWatcher + +* `path` {string|Buffer|URL} +* `flags` {string|number} +* `mode` {integer} **Default:** `0o666` +* `callback` {Function} + * `err` {Error} + * `fd` {[fs.FD][#fs_class_fs_fd]} + +Asynchronous file open that returns an `fs.FD` object. See open(2). + +The `flags` argument can be: + +* `'r'` - Open file for reading. +An exception occurs if the file does not exist. + +* `'r+'` - Open file for reading and writing. +An exception occurs if the file does not exist. + +* `'rs+'` - Open file for reading and writing in synchronous mode. Instructs + the operating system to bypass the local file system cache. + + This is primarily useful for opening files on NFS mounts as it allows skipping + the potentially stale local cache. It has a very real impact on I/O + performance so using this flag is not recommended unless it is needed. + + Note that this doesn't turn `fs.openFD()` into a synchronous blocking call. + If synchronous operation is desired `fs.openFDSync()` should be used. + +* `'w'` - Open file for writing. +The file is created (if it does not exist) or truncated (if it exists). + +* `'wx'` - Like `'w'` but fails if `path` exists. + +* `'w+'` - Open file for reading and writing. +The file is created (if it does not exist) or truncated (if it exists). + +* `'wx+'` - Like `'w+'` but fails if `path` exists. + +* `'a'` - Open file for appending. +The file is created if it does not exist. + +* `'ax'` - Like `'a'` but fails if `path` exists. + +* `'a+'` - Open file for reading and appending. +The file is created if it does not exist. + +* `'ax+'` - Like `'a+'` but fails if `path` exists. + +`mode` sets the file mode (permission and sticky bits), but only if the file was +created. It defaults to `0o666` (readable and writable). + +The callback gets two arguments `(err, fd)`. + +The exclusive flag `'x'` (`O_EXCL` flag in open(2)) ensures that `path` is newly +created. On POSIX systems, `path` is considered to exist even if it is a symlink +to a non-existent file. The exclusive flag may or may not work with network file +systems. + +`flags` can also be a number as documented by open(2); commonly used constants +are available from `fs.constants`. On Windows, flags are translated to +their equivalent ones where applicable, e.g. `O_WRONLY` to `FILE_GENERIC_WRITE`, +or `O_EXCL|O_CREAT` to `CREATE_NEW`, as accepted by CreateFileW. + +On Linux, positional writes don't work when the file is opened in append mode. +The kernel ignores the position argument and always appends the data to +the end of the file. + +*Note*: The behavior of `fs.openFD()` is platform-specific for some flags. As +such, opening a directory on macOS and Linux with the `'a+'` flag - see example +below - will return an error. In contrast, on Windows and FreeBSD, a file +descriptor will be returned. + +```js +// macOS and Linux +fs.openFD('', 'a+', (err, fd) => { + // => [Error: EISDIR: illegal operation on a directory, open ] +}); + +// Windows and FreeBSD +fs.openFD('', 'a+', (err, fd) => { + // => null, +}); +``` + +Some characters (`< > : " / \ | ? *`) are reserved under Windows as documented +by [Naming Files, Paths, and Namespaces][]. Under NTFS, if the filename contains +a colon, Node.js will open a file system stream, as described by +[this MSDN page][MSDN-Using-Streams]. + +*Note:* On Windows, opening an existing hidden file using the `w` flag (e.g. +using `fs.openFD()`) will fail with `EPERM`. Existing hidden +files can be opened for writing with the `r+` flag. A call to `fs.ftruncate()` +can be used to reset the file contents. + +## fs.openFDSync(path, flags[, mode]) + + +* `path` {string|Buffer|URL} +* `flags` {string|number} +* `mode` {integer} **Default:** `0o666` +* Returns: {[fs.FD][#fs_class_fs_fd]} + +Synchronous version of [`fs.openFD()`][]. Returns an `fs.FD` object representing +the file descriptor. + ## fs.openSync(path, flags[, mode]) -An `fs.FD` object is a wrapper for a numeric file descriptor. Instances of -`fs.FD` are distinct from numeric file descriptors in that, if the `fs.FD` is -not explicitly closed using the `fd.close()` method, they will automatically -close the file descriptor and will emit a process warning, thereby helping to -prevent memory leaks. +A `fs.FileHandle` object is a wrapper for a numeric file descriptor. Instances +of `fs.FileHandle` are distinct from numeric file descriptors in that, if the +`fs.FileHandle` is not explicitly closed using the `fd.close()` method, they +will automatically close the file descriptor and will emit a process warning, +thereby helping to prevent memory leaks. -Instances of the `fs.FD` object are created internally by the `fs.openFD()` -and `fs.openFDSync()` methods. +Instances of the `fs.FileHandle` object are created internally by the +`fs.openFileHandle()` and `fs.openFileHandleSync()` methods. -### fd.close() +### filehandle.close() @@ -316,18 +316,18 @@ added: REPLACEME Closes the file descriptor. ```js -const fd = fs.openFDSync('thefile.txt', 'r'); -fd.close() +const filehandle = fs.openFileHandleSync('thefile.txt', 'r'); +filehandle.close() .then(() => console.log('ok')) .catch(() => console.log('not ok')); ``` -### fd.fd +### filehandle.fd -Value: {number} The numeric file descriptor managed by the `FD` object. +Value: {number} The numeric file descriptor managed by the `FileHandle` object. ## Class: fs.FSWatcher @@ -2117,9 +2117,9 @@ added: REPLACEME * `mode` {integer} **Default:** `0o666` * `callback` {Function} * `err` {Error} - * `fd` {[fs.FD][#fs_class_fs_fd]} + * `fd` {[fs.FileHandle][#fs_class_fs_filehandle]} -Asynchronous file open that returns an `fs.FD` object. See open(2). +Asynchronous file open that returns a `fs.FileHandle` object. See open(2). The `flags` argument can be: @@ -2136,8 +2136,9 @@ An exception occurs if the file does not exist. the potentially stale local cache. It has a very real impact on I/O performance so using this flag is not recommended unless it is needed. - Note that this doesn't turn `fs.openFD()` into a synchronous blocking call. - If synchronous operation is desired `fs.openFDSync()` should be used. + Note that this doesn't turn `fs.openFileHAndle()` into a synchronous blocking + call. If synchronous operation is desired `fs.openFileHandleSync()` should be + used. * `'w'` - Open file for writing. The file is created (if it does not exist) or truncated (if it exists). @@ -2162,7 +2163,7 @@ The file is created if it does not exist. `mode` sets the file mode (permission and sticky bits), but only if the file was created. It defaults to `0o666` (readable and writable). -The callback gets two arguments `(err, fd)`. +The callback gets two arguments `(err, filehandle)`. The exclusive flag `'x'` (`O_EXCL` flag in open(2)) ensures that `path` is newly created. On POSIX systems, `path` is considered to exist even if it is a symlink @@ -2178,20 +2179,20 @@ On Linux, positional writes don't work when the file is opened in append mode. The kernel ignores the position argument and always appends the data to the end of the file. -*Note*: The behavior of `fs.openFD()` is platform-specific for some flags. As -such, opening a directory on macOS and Linux with the `'a+'` flag - see example -below - will return an error. In contrast, on Windows and FreeBSD, a file -descriptor will be returned. +*Note*: The behavior of `fs.openFileHandle()` is platform-specific for some +flags. As such, opening a directory on macOS and Linux with the `'a+'` flag - +see example below - will return an error. In contrast, on Windows and FreeBSD, +a file descriptor will be returned. ```js // macOS and Linux -fs.openFD('', 'a+', (err, fd) => { +fs.openFileHandle('', 'a+', (err, filehandle) => { // => [Error: EISDIR: illegal operation on a directory, open ] }); // Windows and FreeBSD -fs.openFD('', 'a+', (err, fd) => { - // => null, +fs.openFileHandle('', 'a+', (err, filehandle) => { + // => null, }); ``` @@ -2201,11 +2202,11 @@ a colon, Node.js will open a file system stream, as described by [this MSDN page][MSDN-Using-Streams]. *Note:* On Windows, opening an existing hidden file using the `w` flag (e.g. -using `fs.openFD()`) will fail with `EPERM`. Existing hidden +using `fs.openFileHandle()`) will fail with `EPERM`. Existing hidden files can be opened for writing with the `r+` flag. A call to `fs.ftruncate()` can be used to reset the file contents. -## fs.openFDSync(path, flags[, mode]) +## fs.openFileHandleSync(path, flags[, mode]) @@ -2213,10 +2214,10 @@ added: REPLACEME * `path` {string|Buffer|URL} * `flags` {string|number} * `mode` {integer} **Default:** `0o666` -* Returns: {[fs.FD][#fs_class_fs_fd]} +* Returns: {[fs.FileHandle][#fs_class_fs_filehandle]} -Synchronous version of [`fs.openFD()`][]. Returns an `fs.FD` object representing -the file descriptor. +Synchronous version of [`fs.openFileHandle()`][]. Returns a `fs.FileHandle` +object representing the file descriptor. ## fs.openSync(path, flags[, mode])