Re: delete-process bug

[michael.cadilhac@lrde.org (Michaël Cadilhac)]

[-- Attachment #1.1.1: Type: text/plain, Size: 2682 bytes --]

michael.cadilhac@lrde.org (Michaël Cadilhac) writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> BTW, did anyone actually test my code to make sure that it does indeed work
>> and fix the problem?
>>
>>         Stefan
>
> As far as I tested, it doesn't work and freeze my Emacs. I'll
> investigate today.

I fixed what we discussed here.

I found two bugs in your proposal:
- the list was set to Qnil at the wrong place (wrong init)
- `return' was used in the handler, but it could still have some
   process to treat (a continue would have been better but still
   wrong).

A patch is attached.

However, the whole thing still doesn't work ; Emacs freezes or prints
this kind of error message: 

Error in post-command-hook: (wrong-type-argument listp (nil DEAD . 18764377))

I thought that the race condition could be the following:
- delq makes some change on  the list but not all,
- It is interrupted by the handler,
- The handler now has to deal with a malformed list.

It's certainly a bug too, as with the delq I had some Abort in the
handler of the form:

#0  0xb7a45951 in kill () from /lib/libc.so.6
#1  0x080eb75c in abort () at emacs.c:464
#2  0x081536b2 in Fsignal (error_symbol=137610425, data=137557813) at eval.c:1616
#3  0x081402ce in wrong_type_argument (predicate=137622913, value=149282605) at data.c:124
#4  0x0815c175 in Fmember (elt=71112, list=149282605) at fns.c:1468
#5  0x0818577c in sigchld_handler (signo=17) at process.c:6390

Which I think I don't have without, however, it is not the only error,
as with this line deleted, I still have the previous error and
problems. For example, I don't think it's normal for emacs to receive
a SIGPIPE

#0  0xb7ba62b8 in write () from /lib/libpthread.so.0
#1  0x00000002 in ?? ()
#2  0x0810561e in emacs_write (fildes=12, buf=0x8d26178 "!\n", nbyte=12) at sysdep.c:3370
#3  0x08187afb in send_process (proc=150054228, buf=0x8d26178 "!\n", len=2, object=149437363) at process.c:5489
#4  0x08188274 in Fprocess_send_string (process=150054228, string=149437363) at process.c:5643
#5  0x081534cb in Ffuncall (nargs=3, args=0xbfea69b4) at eval.c:2905

... Alas, most of the backtraces only show Emacs blocked in select, 
pthread things or wait_for_termination.

If any hacker can help :-)


Maybe it's  not the good solution  to keep traces of  these. The other
possibility (if we  don't consider the wait_for_termination solution),
is to store the PID of the synchronous process ; but I'm confused with
compatibilities with DOS and Mac :-/

If we have the PID of the synchronous process, we can explicitly test
it in sigchld_handler.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: process.patch --]
[-- Type: text/x-patch, Size: 6576 bytes --]

Index: src/process.c
===================================================================
RCS file: /sources/emacs/emacs/src/process.c,v
retrieving revision 1.481
diff -c -r1.481 process.c
*** src/process.c	8 May 2006 05:19:42 -0000	1.481
--- src/process.c	28 May 2006 15:29:27 -0000
***************
*** 778,783 ****
--- 778,792 ----
    return proc;
  }
  
+ 
+ /* Fdelete_process promises to immediately forget about the process, but in
+    reality, Emacs needs to remember those processes until they have been
+    treated by sigchld_handler; otherwise this handler would consider the
+    process as being synchronous and say that the synchronous process is
+    dead.  */
+ static Lisp_Object deleted_pid_list;
+ 
+ 
  DEFUN ("delete-process", Fdelete_process, Sdelete_process, 1, 1, 0,
         doc: /* Delete PROCESS: kill it and forget about it immediately.
  PROCESS may be a process, a buffer, the name of a process or buffer, or
***************
*** 800,805 ****
--- 809,818 ----
    else if (XINT (p->infd) >= 0)
      {
        Fkill_process (process, Qnil);
+       /* No problem storing the pid here, as it is still in Vprocess_alist.  */
+       deleted_pid_list = Fcons (make_fixnum_or_float (p->pid),
+ 				/* GC handled elements set to nil.  */
+ 				Fdelq (Qnil, deleted_pid_list));
        /* Do this now, since remove_process will make sigchld_handler do nothing.  */
        p->status
  	= Fcons (Qsignal, Fcons (make_number (SIGKILL), Qnil));
***************
*** 6373,6445 ****
  
        /* Find the process that signaled us, and record its status.  */
  
!       p = 0;
!       for (tail = Vprocess_alist; GC_CONSP (tail); tail = XCDR (tail))
! 	{
! 	  proc = XCDR (XCAR (tail));
! 	  p = XPROCESS (proc);
! 	  if (GC_EQ (p->childp, Qt) && p->pid == pid)
! 	    break;
  	  p = 0;
! 	}
  
!       /* Look for an asynchronous process whose pid hasn't been filled
! 	 in yet.  */
!       if (p == 0)
! 	for (tail = Vprocess_alist; GC_CONSP (tail); tail = XCDR (tail))
! 	  {
! 	    proc = XCDR (XCAR (tail));
! 	    p = XPROCESS (proc);
! 	    if (p->pid == -1)
! 	      break;
! 	    p = 0;
! 	  }
  
!       /* Change the status of the process that was found.  */
!       if (p != 0)
! 	{
! 	  union { int i; WAITTYPE wt; } u;
! 	  int clear_desc_flag = 0;
  
! 	  XSETINT (p->tick, ++process_tick);
! 	  u.wt = w;
! 	  p->raw_status = u.i;
! 	  p->raw_status_new = 1;
! 
! 	  /* If process has terminated, stop waiting for its output.  */
! 	  if ((WIFSIGNALED (w) || WIFEXITED (w))
! 	      && XINT (p->infd) >= 0)
! 	    clear_desc_flag = 1;
  
! 	  /* We use clear_desc_flag to avoid a compiler bug in Microsoft C.  */
! 	  if (clear_desc_flag)
! 	    {
! 	      FD_CLR (XINT (p->infd), &input_wait_mask);
! 	      FD_CLR (XINT (p->infd), &non_keyboard_wait_mask);
! 	    }
  
! 	  /* Tell wait_reading_process_output that it needs to wake up and
! 	     look around.  */
! 	  if (input_available_clear_time)
! 	    EMACS_SET_SECS_USECS (*input_available_clear_time, 0, 0);
! 	}
  
! 	/* There was no asynchronous process found for that id.  Check
! 	   if we have a synchronous process.  */
!       else
! 	{
! 	  synch_process_alive = 0;
  
! 	  /* Report the status of the synchronous process.  */
! 	  if (WIFEXITED (w))
! 	    synch_process_retcode = WRETCODE (w);
! 	  else if (WIFSIGNALED (w))
!             synch_process_termsig = WTERMSIG (w);
! 
! 	  /* Tell wait_reading_process_output that it needs to wake up and
! 	     look around.  */
! 	  if (input_available_clear_time)
! 	    EMACS_SET_SECS_USECS (*input_available_clear_time, 0, 0);
  	}
  
        /* On some systems, we must return right away.
--- 6386,6466 ----
  
        /* Find the process that signaled us, and record its status.  */
  
!       /* The process can have been deleted by Fdelete_process.  */
!       tail = Fmember (make_fixnum_or_float (pid), deleted_pid_list);
!       if (!NILP (tail))
! 	Fsetcar (tail, Qnil);
!       else
!         {
! 	  /* Otherwise, if it is asynchronous, it is in Vprocess_alist.  */
  	  p = 0;
! 	  for (tail = Vprocess_alist; GC_CONSP (tail); tail = XCDR (tail))
! 	    {
! 	      proc = XCDR (XCAR (tail));
! 	      p = XPROCESS (proc);
! 	      if (GC_EQ (p->childp, Qt) && p->pid == pid)
! 		break;
! 	      p = 0;
! 	    }
  
! 	  /* Look for an asynchronous process whose pid hasn't been filled
! 	     in yet.  */
! 	  if (p == 0)
! 	    for (tail = Vprocess_alist; GC_CONSP (tail); tail = XCDR (tail))
! 	      {
! 		proc = XCDR (XCAR (tail));
! 		p = XPROCESS (proc);
! 		if (p->pid == -1)
! 		  break;
! 		p = 0;
! 	      }
  
! 	  /* Change the status of the process that was found.  */
! 	  if (p != 0)
! 	    {
! 	      union { int i; WAITTYPE wt; } u;
! 	      int clear_desc_flag = 0;
  
! 	      XSETINT (p->tick, ++process_tick);
! 	      u.wt = w;
! 	      p->raw_status = u.i;
! 	      p->raw_status_new = 1;
! 
! 	      /* If process has terminated, stop waiting for its output.  */
! 	      if ((WIFSIGNALED (w) || WIFEXITED (w))
! 		  && XINT (p->infd) >= 0)
! 		clear_desc_flag = 1;
  
! 	      /* We use clear_desc_flag to avoid a compiler bug in Microsoft C.  */
! 	      if (clear_desc_flag)
! 		{
! 		  FD_CLR (XINT (p->infd), &input_wait_mask);
! 		  FD_CLR (XINT (p->infd), &non_keyboard_wait_mask);
! 		}
  
! 	      /* Tell wait_reading_process_output that it needs to wake up and
! 		 look around.  */
! 	      if (input_available_clear_time)
! 		EMACS_SET_SECS_USECS (*input_available_clear_time, 0, 0);
! 	    }
  
! 	  /* There was no asynchronous process found for that id.  Check
! 	     if we have a synchronous process.  */
! 	  else
! 	    {
! 	      synch_process_alive = 0;
  
! 	      /* Report the status of the synchronous process.  */
! 	      if (WIFEXITED (w))
! 		synch_process_retcode = WRETCODE (w);
! 	      else if (WIFSIGNALED (w))
! 		synch_process_termsig = WTERMSIG (w);
! 
! 	      /* Tell wait_reading_process_output that it needs to wake up and
! 		 look around.  */
! 	      if (input_available_clear_time)
! 		EMACS_SET_SECS_USECS (*input_available_clear_time, 0, 0);
! 	    }
  	}
  
        /* On some systems, we must return right away.
***************
*** 6843,6848 ****
--- 6864,6870 ----
    FD_SET (0, &input_wait_mask);
  
    Vprocess_alist = Qnil;
+   deleted_pid_list = Qnil;
    for (i = 0; i < MAXDESC; i++)
      {
        chan_process[i] = Qnil;

[-- Attachment #1.1.3: Type: text/plain, Size: 325 bytes --]

  
-- 
 |      Michaël `Micha' Cadilhac   |   Mieux vaut se taire                  |
 |         Epita/LRDE Promo 2007   |    Que de parler trop fort.            |
 | http://www.lrde.org/~cadilh_m   |            -- As de trèfle             |
 `--  -   JID: micha@amessage.be --'                                   -  --'

[-- Attachment #1.2: Type: application/pgp-signature, Size: 188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel