Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing fs(/promises) APIs: dup, dup2, fdopen #41733

Closed
zackw opened this issue Jan 28, 2022 · 8 comments
Closed

Missing fs(/promises) APIs: dup, dup2, fdopen #41733

zackw opened this issue Jan 28, 2022 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.

Comments

@zackw
Copy link

zackw commented Jan 28, 2022

What is the problem this feature will solve?

The fs module exposes most of the OS-level file operations one would expect, but a few are missing, and the missing operations that I particularly need today are dup and dup2.

In addition, fs/promises does not currently offer a way to wrap a FileHandle around an OS-level file descriptor. I suggest that the name for this operation should be fdopen, matching C/POSIX.

The use case I have in mind today for all three of these functions is for programs that, in the Unix tradition, generate data and write it to process.stdout. Simply calling process.stdout.write or readable.pipe(process.stdout) has two problems today. First, this can be extremely inefficient because process.stdout does no buffering. Second, several methods of the global console object write to process.stdout, so third-party libraries that call e.g. console.info (perhaps only rarely) can corrupt your output, and there doesn't appear to be any official way to reconfigure the console object to not do that. If fs had dup, dup2, and fdopen, I could work around these problems with a function like

import * as fs from "fs/promises";
import process from "process";
async function prepare_buffered_stdout(options) {
    options ??= {};
    const stdout_copy = await fs.dup(process.stdout.fd);
    await fs.dup2(process.stderr.fd, process.stdout.fd);
    return fs.fdopen(stdout_copy, "w").createWriteStream(options);
}

This rearranges things at the level of file descriptors such that output to process.stdout (including via the console) is now sent to process.stderr, and then returns a normal (in particular, buffering) WriteStream that sends data to the original process.stdout. The data generator can be piped to this stream, and it can be closed without causing problems for unrelated code that assumes process.stdout is always open.

Other use cases for fsPromises.fdopen are common: any time a node.js program receives an OS-level file descriptor from external code, the first thing it probably wants to do with it is wrap it in a FileHandle. Just off the top of my head, existing APIs that might hand a node.js program a bare file descriptor include

  • systemd's sd_listen_fds mechanism: server processes are started with extra fds open (beyond stdin/out/err) and are expected to retrieve these fds and use them as listening sockets
  • The Wayland clipboard protocol involves transferring a file descriptor from one client to another (via a SCM_RIGHTS special socket message).

Use cases for dup and dup2 are more unusual but do exist, particularly in low-level code.

What is the feature you are proposing to solve the problem?

Add the following functions to the fs/promises module. TypeScript notation used to document function arguments and return types:

/** Duplicate an OS-level file descriptor.
 *  By default the new fd is marked close-on-exec.
 *  Supply { closeOnExec: false } as options to have it not be close-on-exec.
 *  [Implementation note: POSIX dup(2) primitive.
 *   fcntl(fd, F_DUPFD[_CLOEXEC]) may also be useful.] 
 */
export async function dup(fd: number, options?: { closeOnExec?: boolean }): number;

/** Duplicate OS-level file descriptor OLDFD.
 *  Assign the duplicate file descriptor number NEWFD,
 *  replacing any existing fd with that number.
 *  By default the new fd is NOT marked close-on-exec,
 *  because a frequent reason to want a specific number for
 *  the duplicate is because you're going to transfer it to
 *  an exec()ed program.
 *  Supply { closeOnExec: true } to have the new fd be close-on-exec.
 *  [Implementation note: POSIX dup2(2) primitive.
 *  Linux's dup3() extension may also be useful.] 
 */
export async function dup2(oldfd: number, newfd: number,
                 options?: { closeOnExec?: boolean }): void;

/** Create a FileHandle object that wraps an existing OS-level file descriptor. */
export function fdopen(fd: number): FileHandle;

A FileHandle.dup method (returning another FileHandle, not a bare fd) would also be appropriate for completeness.

FileHandle.__prototype__.dup = async function (options) {
    return fdopen(await dup(this.fd, options ?? {}));
}

What alternatives have you considered?

The fs.createReadStream and fs.createWriteStream functions in the old fs module offer a 'fd' option; this makes them sufficient for many situations where fsPromises.fdopen would be useful, However, they involve using the old fs module rather than fs/promises, and FileHandle objects have many other handy features that people might want.

My specific use case for dup and dup2 could be addressed directly in the standard library -- essentially, add prepare_buffered_stdout instead of the primitives that would make it possible for me to implement it myself. However, that solves only that problem and not any of the other reasons people might want the primitives.

@zackw zackw added the feature request Issues that request new features to be added to Node.js. label Jan 28, 2022
@Mesteery Mesteery added the fs Issues and PRs related to the fs subsystem / file system. label Jan 28, 2022
@mscdex
Copy link
Contributor

mscdex commented Jan 28, 2022

Support for these would need to land in libuv first.

@zackw
Copy link
Author

zackw commented Jan 28, 2022

Hm, I can see that for dup and dup2, but fdopen? Should I have filed a separate request for fdopen?

@mscdex
Copy link
Contributor

mscdex commented Jan 28, 2022

All of them would need to be exposed by libuv and would need to deal with any platform-specific quirks. For example, all 3 of those functions exist on Windows, but they are deprecated aliases and so the underscore-prefixed versions of the functions should be used instead. There could be more platform-specific differences, but any/all of them should be dealt with at the libuv level.

@zackw
Copy link
Author

zackw commented Jan 28, 2022

I'm confused why fsPromises.fdopen would need to use the C fdopen. I'm only proposing Node reuse the name. Looking at the implementation of fsPromises.open, I believe a correct implementation of fsPromises.fdopen would be

static void FdopenFileHandle(const FunctionCallbackInfo<Value>& args) {
  BindingData* binding_data = Environment::GetBindingData<BindingData>(args);

  const int argc = args.Length();
  CHECK_GE(argc, 1);

  CHECK(args[0]->IsInt32());
  const int fd = args[0].As<Int32>()->Value();
  CHECK_GE(fd, 0);

  FileHandle* fh = FileHandle::New(binding_data, fd);
  if (fh == nullptr) return;
  args.GetReturnValue().Set(fh->object());
}

(note minor correction to originally proposed API - no 'mode' argument).

I filed libuv/libuv#3448 for uv support for dup and dup2.

@zackw
Copy link
Author

zackw commented Jan 28, 2022

Hang on, given the existence of a void FileHandle::New(const FunctionCallbackInfo<Value>& args) overload at the C level, is it actually possible for one to write this at the JS level?

import process from 'process';
import { FileHandle } from 'fs/promises';
const fh = new FileHandle(process.stdout.fd);  // for instance

It doesn't work with the copy of Node on the computer where I'm typing this, but that might just be because it's too old (v12). If this works, then it does what I'd want fs.fdopen to do, it just needs to be documented.

@mscdex
Copy link
Contributor

mscdex commented Jan 29, 2022

fdopen() probably doesn't make sense to add since it returns a FILE*, which neither libuv or node would ever utilize and we should try not to expose APIs named after existing C-based APIs to avoid confusion.

You can already pass a file descriptor to various fs APIs, so adding support for FileHandle or other promises-based fs APIs using new signatures or options would be the best way forward if support is currently missing in those areas.

Adding dup() and dup2() however make sense as such functionality can't really be folded into existing node APIs and the functions are not dealing with any special value types and thus could easily be supported once libuv supports them.

@milahu
Copy link

milahu commented Jul 3, 2022

workaround on linux

var fdDup = fs.openSync(`/proc/self/fd/0`); // os.dup(0) on linux

use case for dup: fs.readSync with timeout

example use in tokenpool-gnu-make-posix.cc (details) (javascript) (python)

what does not work
//var stdinDup = os.dup(0);
var stdinDup = fs.openSync(`/proc/self/fd/0`);
var closeTimer = setTimeout(() => stdinDup.close(), 100);
var buffer = Buffer.alloc(1);
try {
  var bytesRead = fs.readSync(stdinDup); // can block until stdinDup.close
  console.log(`done reading ${bytesRead} bytes from stdin: int ${buffer.readInt8()`);
}
catch (e) {
  if (e.errno == -11) {
    console.log("stdin is empty");
  }
  else throw e;
}
clearTimeout(closeTimer);

... but closeTimer is never called

cat | node -e '
  var fs = require("fs");
  var process = require("process");
  var buf = Buffer.alloc(1);
  var fdNum = parseInt(process.argv[1]);
  var fdDup = fs.openSync(`/proc/self/fd/${fdNum}`); // os.dup on linux
  var closeTimer = setTimeout(() => {
    console.error("timeout");
    fdDup.close();
  }, 1000);
  try {
    console.error(`read ...`);
    fs.readSync(fdDup, buf);
    console.error(`read done`);
    clearTimeout(closeTimer);
    console.log(buf.readInt8());
  } catch (e) {
    clearTimeout(closeTimer);
    console.error(`error ${e}`);
  }
' 0

hangs after read ...

@bnoordhuis
Copy link
Member

I closed out libuv/libuv#3448 just now. I'll go ahead and close out this issue as well.

If someone still wants to pursue this it's probably better to create a new issue because the discussion in this one is somewhat confusing and meandering.

FWIW, dup() and dup2() have rather subtle semantics (file descriptors vs. file descriptions) and I don't necessarily agree it's a good idea to add them to node.


@milahu Try opening the file with fs.openSync(/proc/self/fd/0, fs.constants.O_RDONLY + fs.constants.O_NONBLOCK) and passing it to fs.createReadStream(null, { fd: ... }).

Not that bypassing node's stdin handling is necessarily a good idea so use at your own risk.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

No branches or pull requests

5 participants