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

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

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

Agustin Martin <agustin.martin@hispalinux.es> 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.


[-- Attachment #1.1.2: process.patch --]
[-- Type: text/x-patch, Size: 1302 bytes --]

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  <michael.cadilhac@lrde.org>
+ 
+ 	* process.c (Fdelete_process): Wait for process termination to
+ 	avoid `sigchld_handler' to consider the process to be synchronous.
+ 
  2006-05-24  Luc Teirlinck  <teirllm@auburn.edu>
  
  	* 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));

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


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