Subject: Re: bug#554: OSX: with-temp-buffer kills unrelated processes

From: YAMAMOTO Mitsuharu <[email-masked]>
Date: Wed, 16 Jul 2008 18:17:52 +0900

>>>>> On Mon, 14 Jul 2008 19:26:52 +0900, YAMAMOTO Mitsuharu <[email-masked]> said:

> It seems that the child process receives SIGHUP before it calls
> execvp.  As the child process is still running the Emacs executable,
> it sends SIGHUP to all the subprocesses when it receives SIGHUP
> (fatal_error_signal -> shut_down_emacs -> kill_buffer_processes).

> I think these behaviors are not observable in other platforms,
> because they use vfork, and the parent side effectively blocks until
> the child side calls execvp or others in its typical
> implementations.  Emacs on Darwin is using fork instead of vfork
> because the latter had (or have?) some problems I'm not familiar
> with.

OK, close-on-exec seems to be usable for the `fork' case.  How about
the patch below?

				     YAMAMOTO Mitsuharu
				[email-masked]

Index: src/callproc.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/callproc.c,v
retrieving revision 1.221.2.3
diff -c -p -r1.221.2.3 callproc.c
*** src/callproc.c	8 Jan 2008 04:30:19 -0000	1.221.2.3
--- src/callproc.c	16 Jul 2008 08:03:57 -0000
*************** static int relocate_fd ();
*** 1192,1199 ****
     Therefore, the superior process must save and restore the value
     of environ around the vfork and the call to this function.
  
!    SET_PGRP is nonzero if we should put the subprocess into a separate
!    process group.
  
     CURRENT_DIR is an elisp string giving the path of the current
     directory the subprocess should have.  Since we can't really signal
--- 1192,1199 ----
     Therefore, the superior process must save and restore the value
     of environ around the vfork and the call to this function.
  
!    RET_ON_FAIL is nonzero if we should return from the function on
!    failure of execvp rather than calling _exit.
  
     CURRENT_DIR is an elisp string giving the path of the current
     directory the subprocess should have.  Since we can't really signal
*************** static int relocate_fd ();
*** 1201,1210 ****
     executable directory by the parent.  */
  
  int
! child_setup (in, out, err, new_argv, set_pgrp, current_dir)
       int in, out, err;
       register char **new_argv;
!      int set_pgrp;
       Lisp_Object current_dir;
  {
    char **env;
--- 1201,1210 ----
     executable directory by the parent.  */
  
  int
! child_setup (in, out, err, new_argv, ret_on_fail, current_dir)
       int in, out, err;
       register char **new_argv;
!      int ret_on_fail;
       Lisp_Object current_dir;
  {
    char **env;
*************** child_setup (in, out, err, new_argv, set
*** 1412,1418 ****
    emacs_write (1, "Can't exec program: ", 20);
    emacs_write (1, new_argv[0], strlen (new_argv[0]));
    emacs_write (1, "\n", 1);
!   _exit (1);
  #endif /* not WINDOWSNT */
  #endif /* not MSDOS */
  }
--- 1412,1419 ----
    emacs_write (1, "Can't exec program: ", 20);
    emacs_write (1, new_argv[0], strlen (new_argv[0]));
    emacs_write (1, "\n", 1);
!   if (!ret_on_fail)
!     _exit (1);
  #endif /* not WINDOWSNT */
  #endif /* not MSDOS */
  }
Index: src/process.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/process.c,v
retrieving revision 1.512.2.12
diff -c -p -r1.512.2.12 process.c
*** src/process.c	27 Feb 2008 15:07:14 -0000	1.512.2.12
--- src/process.c	16 Jul 2008 08:03:58 -0000
*************** create_process (process, new_argv, curre
*** 1842,1847 ****
--- 1842,1850 ----
    int inchannel, outchannel;
    pid_t pid;
    int sv[2];
+ #if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+   int wait_child_setup[2];
+ #endif
  #ifdef POSIX_SIGNALS
    sigset_t procmask;
    sigset_t blocked;
*************** create_process (process, new_argv, curre
*** 1884,1895 ****
  #endif
        if (forkin < 0)
  	report_file_error ("Opening pty", Qnil);
- #if defined (RTU) || defined (UNIPLUS) || defined (DONT_REOPEN_PTY)
-       /* In the case that vfork is defined as fork, the parent process
- 	 (Emacs) may send some data before the child process completes
- 	 tty options setup.  So we setup tty before forking.  */
-       child_setup_tty (forkout);
- #endif /* RTU or UNIPLUS or DONT_REOPEN_PTY */
  #else
        forkin = forkout = -1;
  #endif /* not USG, or USG_SUBTTY_WORKS */
--- 1887,1892 ----
*************** create_process (process, new_argv, curre
*** 1924,1929 ****
--- 1921,1945 ----
      }
  #endif /* not SKTPAIR */
  
+ #if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+     {
+       int tem;
+ 
+       tem = pipe (wait_child_setup);
+       if (tem < 0)
+ 	report_file_error ("Creating pipe", Qnil);
+       tem = fcntl (wait_child_setup[1], F_GETFD, 0);
+       if (tem >= 0)
+ 	tem = fcntl (wait_child_setup[1], F_SETFD, tem | FD_CLOEXEC);
+       if (tem < 0)
+ 	{
+ 	  emacs_close (wait_child_setup[0]);
+ 	  emacs_close (wait_child_setup[1]);
+ 	  report_file_error ("Setting file descriptor flags", Qnil);
+ 	}
+     }
+ #endif
+ 
  #if 0
    /* Replaced by close_process_descs */
    set_exclusive_use (inchannel);
*************** create_process (process, new_argv, curre
*** 2174,2189 ****
  #endif /* SIGCHLD */
  #endif /* !POSIX_SIGNALS */
  
- #if !defined (RTU) && !defined (UNIPLUS) && !defined (DONT_REOPEN_PTY)
  	if (pty_flag)
  	  child_setup_tty (xforkout);
- #endif /* not RTU and not UNIPLUS and not DONT_REOPEN_PTY */
  #ifdef WINDOWSNT
  	pid = child_setup (xforkin, xforkout, xforkout,
  			   new_argv, 1, current_dir);
  #else  /* not WINDOWSNT */
  	child_setup (xforkin, xforkout, xforkout,
  		     new_argv, 1, current_dir);
  #endif /* not WINDOWSNT */
        }
      environ = save_environ;
--- 2190,2210 ----
  #endif /* SIGCHLD */
  #endif /* !POSIX_SIGNALS */
  
  	if (pty_flag)
  	  child_setup_tty (xforkout);
  #ifdef WINDOWSNT
  	pid = child_setup (xforkin, xforkout, xforkout,
  			   new_argv, 1, current_dir);
  #else  /* not WINDOWSNT */
+ #ifdef FD_CLOEXEC
+ 	emacs_close (wait_child_setup[0]);
+ #endif
  	child_setup (xforkin, xforkout, xforkout,
  		     new_argv, 1, current_dir);
+ #ifdef FD_CLOEXEC
+ 	emacs_close (wait_child_setup[1]);
+ #endif
+ 	_exit (1);
  #endif /* not WINDOWSNT */
        }
      environ = save_environ;
*************** create_process (process, new_argv, curre
*** 2235,2240 ****
--- 2256,2276 ----
        else
  #endif
  	XPROCESS (process)->tty_name = Qnil;
+ 
+ #if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+       /* Wait for child_setup to complete in case that vfork is
+ 	 actually defined as fork.  The descriptor wait_child_setup[1]
+ 	 of a pipe is closed at the child side either by close-on-exec
+ 	 on successful execvp or by the explicit emacs_close call
+ 	 before _exit above.  */
+       {
+ 	char dummy;
+ 
+ 	emacs_close (wait_child_setup[1]);
+ 	emacs_read (wait_child_setup[0], &dummy, 1);
+ 	emacs_close (wait_child_setup[0]);
+       }
+ #endif
      }
  
    /* Restore the signal state whether vfork succeeded or not.