delete-process bug (was: Ispell loads dict twice.)

[Michaël Cadilhac]

Agustin Martin <address@hidden> writes:

> Ispell process killed
> Starting new Ispell process [american] ...
> Error in post-command-hook: (error ispell exited with signal Killed)

This is  not really  due to my  patch. However,  it shows a  real race
condition in process management of Emacs:

Try the following:

(delete-process (start-process "bug" nil "/tmp/sleep"))
(let ((result (call-process "/tmp/sleep")))
  (message "%S" result))

(/tmp/sleep is just "sleep 1")

You'll have, as a result, the message « Killed » ; but the process
called by call-process is _not actually killed_, it's the previous
(deleted) one that was.

The bug is simple:
- The first process is launched asynchronously, then killed
- A synchronous process is created and executed
- HERE, sigchld_handler is called, because of the first process death
- The PID of the first process is searched in:
  - The list of asynchronous process, in which it is no more,
  - As a fall back, it is considered as a synchronous process.
- Then the synchronous process is said to be killed.

... but it isn't.

After an hour of debugging, I can propose a small change that fixes
this bug and lets no room for any other race condition of that kind,
AFAICT.

Index: src/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/src/ChangeLog,v
retrieving revision 1.5087
diff -c -B -b -r1.5087 ChangeLog
*** src/ChangeLog       24 May 2006 16:58:47 -0000      1.5087
--- src/ChangeLog       25 May 2006 10:55:02 -0000
***************
*** 1,3 ****
--- 1,8 ----
+ 2006-05-25  Michaël Cadilhac  <address@hidden>
+ 
+       * process.c (Fdelete_process): Wait for process termination to
+       avoid `sigchld_handler' to consider the process to be synchronous.
+ 
  2006-05-24  Luc Teirlinck  <address@hidden>
  
        * puresize.h (BASE_PURESIZE): Increase to 1210000.
Index: src/process.c
===================================================================
RCS file: /sources/emacs/emacs/src/process.c,v
retrieving revision 1.481
diff -c -B -b -r1.481 process.c
*** src/process.c       8 May 2006 05:19:42 -0000       1.481
--- src/process.c       25 May 2006 10:55:02 -0000
***************
*** 800,805 ****
--- 800,806 ----
    else if (XINT (p->infd) >= 0)
      {
        Fkill_process (process, Qnil);
+       wait_for_termination (p->pid);
        /* Do this now, since remove_process will make sigchld_handler do 
nothing.  */
        p->status
        = Fcons (Qsignal, Fcons (make_number (SIGKILL), Qnil));

Regards,

-- 
 |      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 --'                                   -  --'

pgpoOhnrH1hPY.pgp
Description: PGP signature