Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Add Flag to Spawn Child Process with New Process Group ID but the Same Session ID #934

Closed
wants to merge 1 commit into from

Conversation

groundwater
Copy link

I am working on a primitive shell in node that has full-fledged job-control. Proper job control requires being able to spawn child processes with the same session id, but in a new process group. Currently libuv only supports flagging a child for complete detachment via UV_PROCESS_DETACHED.

A bit on TTYs and why this is necessary. I apologize if this is all familiar.

Background

Whenever you log into a shell, such as bash, there is always a controlling TTY for the session. That controlling TTY is just a file descriptor for a character device e.g. /dev/console. The reason you see the output from a command like ls is because after bash forks, it keeps STDOUT/STDERR attached to the controlling TTY before calling exec on ls. Thus the TTY is being shared by almost every process that is a direct or indirect child of your login session.

In order to calm the madness, the TTY drive has the concept of a foreground process group. Only processes in the foreground process group are allowed to read and write to the TTYs character device. If a background process tries to read or write to the TTY it will be sent SIGTTIN or SIGTTOUT. The SIGTTOUT signal can be ignored, but ignoring SIGTTIN will result in an EIO read error.

The foreground process group also lets the line-discipline decide which processes should be sent control signals like SIGINT whenever it processes a ^C character.

Problem

The problem is that the system call (tcsetpgrp) for setting foreground process groups can only be done on process groups in the same session. Thus using UV_PROCESS_DETACHED will cause tcsetpgrp to error.

@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit groundwater/libuv@3c99301 has the following error(s):

  • First line of commit message must be no longer than 50 characters
  • Commit message must indicate the subsystem this commit changes

The following commiters were not found in the CLA:

  • Jacob Groundwater

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@@ -280,6 +280,9 @@ static void uv__process_child_init(const uv_process_options_t* options,

if (options->flags & UV_PROCESS_DETACHED)
setsid();

if (options->flags & UV_PROCESS_SETPGID)
setpgid(0,0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: space after comma.

@bnoordhuis
Copy link
Contributor

LGTM I think but a regression test would be nice.

@bb010g
Copy link

bb010g commented Jul 20, 2016

bump

@saghul
Copy link
Contributor

saghul commented Jul 21, 2016

Please reopen at https://github.com/libuv/libuv

@saghul saghul closed this Jul 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants