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.

Sign up for GitHub

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

Jump to bottom

Implement low-level (OS\) subprocess API #176

Open
1 of 10 tasks
fredemmott opened this issue Dec 16, 2021 · 1 comment
Open
1 of 10 tasks

Implement low-level (OS\) subprocess API #176

fredemmott opened this issue Dec 16, 2021 · 1 comment

Comments

@fredemmott
Copy link
Contributor

fredemmott commented Dec 16, 2021 •
edited by Atry
Loading

  • finalize signature of OS\posix_spawn() and OS\posix_spawnp()
  • (in progress, fred) add _OS\dup()
  • add _OS\waitpid_async() or similar
  • add _OS\shutdown()
  • add public APIs:
    • OS\posix_spawn() and OS\posix_spawnp()
    • OS\waitpid_async()
    • OS\shutdown()
    • OS\dup()
    • OS\socketpair() (already in _OS\)
@fredemmott
Copy link
Contributor Author

add _OS\waitpid_async() or similar

Ideally, I'd want to add _OS\waitid_async(), and expose it as OS\waitid_async() and OS\waitpid_async() - however, waitid() does not exist in macos, despite being part of the specifications for some time.

I think we'd want to just add waitpid_async(), and if later we want process group support, we add _OS\waitpgid_async(), and add an OS\waitid_async() that wraps the two separate functions - unless MacOS adds waitid in the mean time.

... but the design of process group support can be left for later, that's just be brain dumping. For now, let's stick with just waitpid.

facebook-github-bot pushed a commit to facebook/hhvm that referenced this issue Mar 21, 2022
@fredemmott @facebook-github-bot
add HH\Lib\_Private\_OS\dup()
ac00a11 
Summary:
refs hhvm/hsl#176

This part of enabling a pattern of:
```
list($stderr_parent, $stderr_child) = OS\socketpair(); // yep, stderr is read-write
$stdin_child = OS\dup($stderr_child);
OS\shutdown($stdin_child, OS\SHUT_WR);
$stdout_child = OS\dup($stderr_child);
OS\shutdown($stdout_child, OS\SHUT_RD);
```

`dup2()` is not necessary as we support explicitly remapping FDs when creating subprocesses

`dup2()` is not desirable as:
- Hack programs do not have insights to the real FD numbers. Especially in CLI server mode, STDIN/STDOUT/STDERR may not be on the canonical STDIN_FILENO/STDOUT_FILENO/STDERR_FILENO
- Given that, there's a large risk of accidentally creating cross-request issues with `dup2()`

Reviewed By: shayne-fletcher

Differential Revision: D33166096

fbshipit-source-id: 41339a9a33c854982009086f3efdfc35277715c0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@fredemmott