Re: delete-process bug

[Michaël Cadilhac]

address@hidden (Michaël Cadilhac) writes:

> Stefan Monnier <address@hidden> 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.

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;

  
-- 
 |      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: address@hidden --'                                   -  --'

pgpEBArewzyjy.pgp
Description: PGP signature