Re: why does NSTask setpgrp on sub processes?

[Derek Zhou]

On Tuesday 19 January 2010 10:14:15 pm Derek Zhou wrote:
> On Tuesday 19 January 2010 12:59:28 am Derek Zhou wrote:
> > hi all,
> > it seems NSTask setpgrp after the fork:
> > ...
> >       /*                                                                    
> >     
> >        * Make sure task is run in it's own process group.                   
> >     
> >        */
> > #ifdef     HAVE_SETPGRP
> > #ifdef  SETPGRP_VOID
> >       setpgrp();
> > #else
> >       setpgrp(getpid(), getpid());
> > #endif
> > 
> > Why is it doing that? Is it simply what Apple is doing?
> > I am trying to call $EDITOR (normally a TTY process) in my program which 
> > has no gui either. 
> > However I suspect the setpgrp mess up the terminal setting so the EDITOR 
> > does not have the tty. I can tcsetpgrp from my process but then my process 
> > is stopped. Can we do this:
> > 0, just don't setpgrp, or
> > 1, don't setpgrp if the parent's stdin is a tty and not setting the child's 
> > stdin, or
> > 2, add an option not to setpgrp. (it is an additional API though)
> > I think 1 is the safest route and I can easily make a patch. I can do 
> > either 0 or 1 if that's what people want.
> > 
> Moving to the dev list. 
> Derek
> 
Here is a simple patch. It just don't setpgrp if the process is on a tty. 
The current behavior does not make sense; if the child try to read/write 
anything on the tty it will be stopped; and I don't think that's what 
people want. If the goal is to prevent the child from messing with the 
tty one can and should redirect stdin/stdout/stderr of the child.
 
Furthermore, I think process group only has something to do with the tty
usage and the signal handling resulting from that, so ideally you do not 
need to call setpgrp unless you are writing a shell or something; it 
is pretty much a no-op in a GUI app or in Windows. Anyway I believe this 
patch is safe and does what I want. 

Derek

Index: Source/NSTask.m
===================================================================
--- Source/NSTask.m     (revision 29320)
+++ Source/NSTask.m     (working copy)
@@ -1387,7 +1387,15 @@
   const char           *envl[ec+1];
   id                   hdl;
   int                  i;
+  BOOL                  shouldSetpgrp = YES;
 
+  /*
+   * do not setpgrp if we are on a tty
+   */
+#ifndef __MINGW32__
+  if (isatty(0))
+    shouldSetpgrp = NO;
+#endif
   if (_hasLaunched)
     {
       [NSException raise: NSInvalidArgumentException
@@ -1473,6 +1481,7 @@
          signal(i, SIG_DFL);
        }
 
+      if (shouldSetpgrp) {
       /*
        * Make sure task is run in it's own process group.
        */
@@ -1492,6 +1501,7 @@
       setpgid(pid, pid);
 #endif
 #endif
+      }
 
       if (_usePseudoTerminal == YES)
        {