bug#15983: 24.3; Emacs Not Killing Child Process

bug#15983: 24.3; Emacs Not Killing Child Process

[sjm]

I'm using nrepl.el for Clojure development and am having trouble with
residual Java processes when quitting nrepl.el.

The process tree that gets spawned looks like this:

emacs.exe
|_ cmdproxy.exe
   |_ cmd.exe
      |_ java.exe
         |_ java.exe

The problem is that after nrepl-quit is called, only the parent java.exe
process is killed and I'm left with an orphaned java.exe that I have to
kill manually.

The code that does the killing looks like this:

(defun nrepl--close-buffer (buffer)
  "Close the nrepl BUFFER."
  (when (get-buffer-process buffer)
    (delete-process (get-buffer-process buffer)))
  (when (get-buffer buffer)
    (kill-buffer buffer)))

The documentation section "37.5 Deleting Processes" says that child
processes get killed but this doesn't seem to be happening for some reason.

I've spoken with the main developer of nrepl.el and he seems to think it
might be a bug in Emacs.

In GNU Emacs 24.3.1 (i386-mingw-nt6.1.7601)
 of 2013-03-17 on MARVIN
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --with-gcc (4.7) --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include
 -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
 -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'

Important settings:
  value of $LANG: ENG
  locale-coding-system: cp1252
  default enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  paredit-mode: t
  shell-dirtrack-mode: t
  global-rainbow-delimiters-mode: t
  rainbow-delimiters-mode: t
  show-paren-mode: t
  desktop-save-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x n r e <return> <help-echo> <down-mouse-1> <mouse-1> 
M-x <right> <return> y C-x o C-x k <return> C-s s m 
t p <return> <down> <home> M-x b u g <right> <right> 
<right> <right> <right> <backspace> <backspace> <backspace> 
r e p o r t - <return> E m a c s SPC n o t SPC k i 
l l i n g SPC c h i l d SPC p r o c e s s <return> 
<down-mouse-1> <mouse-1> C-x k <return> y e s <return> 
C-x 1 <prior> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <return> 
<S-insert> C-x C-s <home> <next> <prior> <up> <up> 
<up> <up> <up> <up> <up> <up> <up> M-x c u s t o m 
<return> e m a <tab> <tab> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> s m 
t <tab> <return> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <next> <prior> q M-x <return> e m a <tab> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
C-g <up> <up> <up> <up> <up> <up> <up> <up> <end> <left> 
<left> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> s j 
m @ s j m . i o C-x C-s <home> C-SPC <down> C-w <down> 
<S-insert> C-x C-s <up> <end> C-x C-e M-x <right> <return> 
E m a c s SPC <help-echo> <help-echo> <down-mouse-1> 
<mouse-1> <help-echo> <help-echo> <help-echo> <end> 
<C-backspace> <S-insert> <return> C-x k <return> y 
e s <return> C-x 1 M-x <return>

Recent messages:
Checking 70 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/erc...
Checking 48 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/emulation...
Checking 147 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/emacs-lisp...
Checking 24 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/cedet...
Checking 57 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/calendar...
Checking 87 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/calc...
Checking 77 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/obsolete...
Checking 2 files in c:/Users/Simon/Desktop/Programs/emacs-24.3/leim...
Checking for load-path shadows...done
Mark activated

Load-path shadows:
~/.emacs.d/bs hides c:/Users/Simon/Desktop/Programs/emacs-24.3/lisp/bs

Features:
(smtpmail cus-edit cus-start cus-load wid-edit help-mode shadow sort
mail-extr emacsbug message rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mail-utils misearch multi-isearch lisp-mnt
network-stream starttls tls smex dired org-wl org-w3m org-vm org-rmail
org-mhe org-mew org-irc org-jsinfo org-infojs org-html org-exp ob-exp
org-exp-blocks org-agenda org-info org-gnus org-docview org-bibtex
bibtex org-bbdb org warnings ob-tangle ob-ref ob-lob ob-table
org-footnote org-src ob-comint ob-keys org-pcomplete org-list org-faces
org-entities org-version ob-emacs-lisp ob org-compat org-macs ob-eval
org-loaddefs cal-menu calendar cal-loaddefs clojure-test-mode nrepl
compile ewoc eldoc arc-mode archive-mode etags pkg-info find-func epl
dash which-func paredit clojure-mode imenu inf-lisp tramp tramp-compat
auth-source eieio byte-opt bytecomp byte-compile cconv gnus-util mm-util
mail-prsvr password-cache tramp-loaddefs cl-macs gv shell pcomplete
format-spec advice help-fns advice-preload cl cl-lib ample-theme edmacro
kmacro rainbow-delimiters paren ido desktop frink-mode comint ansi-color
ring markdown-mode thingatpt noutline outline easy-mmode easymenu
ample-theme-autoloads clojure-test-mode-autoloads
markdown-mode-autoloads nrepl-autoloads clojure-mode-autoloads
paredit-autoloads pkg-info-autoloads epl-autoloads finder-inf
dash-autoloads rainbow-delimiters-autoloads s-autoloads smex-autoloads
package time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel
dos-w32 ls-lisp w32-common-fns disp-table w32-win w32-vars tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process w32 multi-tty
emacs)

bug#15983: 24.3; Emacs Not Killing Child Process

[Eli Zaretskii]

> From: sjm@sjm.io
> Date: Wed, 27 Nov 2013 17:47:38 +0000
> 
> I'm using nrepl.el for Clojure development and am having trouble with
> residual Java processes when quitting nrepl.el.
> 
> The process tree that gets spawned looks like this:
> 
> emacs.exe
> |_ cmdproxy.exe
>    |_ cmd.exe
>       |_ java.exe
>          |_ java.exe
> 
> The problem is that after nrepl-quit is called, only the parent java.exe
> process is killed and I'm left with an orphaned java.exe that I have to
> kill manually.
> 
> The code that does the killing looks like this:
> 
> (defun nrepl--close-buffer (buffer)
>   "Close the nrepl BUFFER."
>   (when (get-buffer-process buffer)
>     (delete-process (get-buffer-process buffer)))
>   (when (get-buffer buffer)
>     (kill-buffer buffer)))
> 
> The documentation section "37.5 Deleting Processes" says that child
> processes get killed but this doesn't seem to be happening for some reason.
> 
> I've spoken with the main developer of nrepl.el and he seems to think it
> might be a bug in Emacs.

Emacs on Windows can only monitor and kill its immediate subprocesses,
it cannot monitor, let alone kill, any of their descendant processes,
because it has no idea about them.  And the OS doesn't automatically
kill all the processes in the subprocess tree, and there's no way to
send a signal to them all, as on Posix platforms.  If killing the
immediate child process doesn't cause some of its children to exit or
abort, then those grandchildren will be left orphaned.

Why doesn't the child java.exe exit when it parent does?  Can you
arrange for that to happen?  Failing that, I don't think there's a
solution to this problem, sorry.

bug#15983: 24.3; Emacs Not Killing Child Process

[Joan Karadimov]

[-- Attachment #1.1: Type: text/plain, Size: 1101 bytes --]

> Emacs on Windows can only monitor and kill its immediate subprocesses,
> it cannot monitor, let alone kill, any of their descendant processes,> because it has no idea about them.  And the OS doesn't automatically> kill all the processes in the subprocess tree, and there's no way to> send a signal to them all, as on Posix platforms.  If killing the> immediate child process doesn't cause some of its children to exit or> abort, then those grandchildren will be left orphaned.
Windows NT does have the concept of parent processes, but an API call
wasn't exposed in win32 until XP. I wrote a small patch that uses that
and kills all child processes (as long as pid<0). I did some sanity
testing and it works.

There are a few things that this patch leaves unaddressed:- there is
no error handling
- there is no OS detection - this will get ugly on Windows 9x and
NT4/5.0- performance: 3 API calls are made for each descendant
process. This can be reduced to a total 3 calls (regardless of the
child process count)

I'll fix these (and any other issues) if this fix is of any interest.

[-- Attachment #1.2: Type: text/html, Size: 1629 bytes --]

[-- Attachment #2: win32-kill-children --]
[-- Type: application/octet-stream, Size: 1850 bytes --]

diff --git src/w32proc.c src/w32proc.c
index 2b583ef..0b82ba3 100644
--- src/w32proc.c
+++ src/w32proc.c
@@ -43,6 +43,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #undef kill
 
 #include <windows.h>
+#include <tlhelp32.h>
 #ifdef __GNUC__
 /* This definition is missing from mingw32 headers. */
 extern BOOL WINAPI IsValidLocale (LCID, DWORD);
@@ -2270,19 +2271,14 @@ find_child_console (HWND hwnd, LPARAM arg)
   return TRUE;
 }
 
-/* Emulate 'kill', but only for other processes.  */
-int
-sys_kill (pid_t pid, int sig)
+static int
+kill_process (pid_t pid, int sig)
 {
   child_process *cp;
   HANDLE proc_hand;
   int need_to_free = 0;
   int rc = 0;
 
-  /* Each process is in its own process group.  */
-  if (pid < 0)
-    pid = -pid;
-
   /* Only handle signals that will result in the process dying */
   if (sig != 0
       && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP)
@@ -2488,6 +2484,36 @@ sys_kill (pid_t pid, int sig)
   return rc;
 }
 
+static int
+kill_process_tree (int pid, int sig)
+{
+  PROCESSENTRY32 process_entry;
+  HANDLE snapshot;
+
+  process_entry.dwSize = sizeof (process_entry);
+  snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPPROCESS, pid);
+
+  kill_process (pid, sig);
+  Process32First (snapshot, &process_entry);
+  do
+    {
+      if (process_entry.th32ParentProcessID == pid)
+        kill_process_tree (process_entry.th32ProcessID, sig);
+    } while (Process32Next (snapshot, &process_entry));
+  CloseHandle (snapshot);
+  return 0;
+}
+
+/* Emulate 'kill', but only for other processes.  */
+int
+sys_kill (int pid, int sig)
+{
+  if (pid < 0)
+    return kill_process_tree (-pid, sig);
+  else
+    return kill_process (pid, sig);
+}
+
 /* The following two routines are used to manipulate stdin, stdout, and
    stderr of our child processes.
 

bug#15983: Fw: bug#15983: 24.3; Emacs Not Killing Child Process

[Bozhidar Batsov]

[-- Attachment #1.1: Type: text/plain, Size: 1389 bytes --]

Forwarded message:
> From: Joan Karadimov <joan.karadimov@gmail.com>
> To: bug-gnu-emacs@gnu.org
> Cc: sjm@sjm.io, eliz@gnu.org, Bozhidar Batsov <bozhidar.batsov@gmail.com>
> Date: Thursday, December 19, 2013 at 5:44:37 PM
> Subject: bug#15983: 24.3; Emacs Not Killing Child Process
> 
> > Emacs on Windows can only monitor and kill its immediate subprocesses, > it cannot monitor, let alone kill, any of their descendant processes, > because it has no idea about them. And the OS doesn't automatically > kill all the processes in the subprocess tree, and there's no way to > send a signal to them all, as on Posix platforms. If killing the > immediate child process doesn't cause some of its children to exit or > abort, then those grandchildren will be left orphaned. Windows NT does have the concept of parent processes, but an API call wasn't exposed in win32 until XP. I wrote a small patch that uses that and kills all child processes (as long as pid<0). I did some sanity testing and it works. 
> There are a few things that this patch leaves unaddressed: - there is no error handling - there is no OS detection - this will get ugly on Windows 9x and NT4/5.0 - performance: 3 API calls are made for each descendant process. This can be reduced to a total 3 calls (regardless of the child process count)
> 
> I'll fix these (and any other issues) if this fix is of any interest.
> 


[-- Attachment #1.2: Type: text/html, Size: 2784 bytes --]

[-- Attachment #2: win32-kill-children --]
[-- Type: application/OCTET-STREAM, Size: 1850 bytes --]

diff --git src/w32proc.c src/w32proc.c
index 2b583ef..0b82ba3 100644
--- src/w32proc.c
+++ src/w32proc.c
@@ -43,6 +43,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #undef kill
 
 #include <windows.h>
+#include <tlhelp32.h>
 #ifdef __GNUC__
 /* This definition is missing from mingw32 headers. */
 extern BOOL WINAPI IsValidLocale (LCID, DWORD);
@@ -2270,19 +2271,14 @@ find_child_console (HWND hwnd, LPARAM arg)
   return TRUE;
 }
 
-/* Emulate 'kill', but only for other processes.  */
-int
-sys_kill (pid_t pid, int sig)
+static int
+kill_process (pid_t pid, int sig)
 {
   child_process *cp;
   HANDLE proc_hand;
   int need_to_free = 0;
   int rc = 0;
 
-  /* Each process is in its own process group.  */
-  if (pid < 0)
-    pid = -pid;
-
   /* Only handle signals that will result in the process dying */
   if (sig != 0
       && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP)
@@ -2488,6 +2484,36 @@ sys_kill (pid_t pid, int sig)
   return rc;
 }
 
+static int
+kill_process_tree (int pid, int sig)
+{
+  PROCESSENTRY32 process_entry;
+  HANDLE snapshot;
+
+  process_entry.dwSize = sizeof (process_entry);
+  snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPPROCESS, pid);
+
+  kill_process (pid, sig);
+  Process32First (snapshot, &process_entry);
+  do
+    {
+      if (process_entry.th32ParentProcessID == pid)
+        kill_process_tree (process_entry.th32ProcessID, sig);
+    } while (Process32Next (snapshot, &process_entry));
+  CloseHandle (snapshot);
+  return 0;
+}
+
+/* Emulate 'kill', but only for other processes.  */
+int
+sys_kill (int pid, int sig)
+{
+  if (pid < 0)
+    return kill_process_tree (-pid, sig);
+  else
+    return kill_process (pid, sig);
+}
+
 /* The following two routines are used to manipulate stdin, stdout, and
    stderr of our child processes.
 

bug#15983: 24.3; Emacs Not Killing Child Process

[Eli Zaretskii]

> Date: Thu, 19 Dec 2013 17:44:37 +0200
> From: Joan Karadimov <joan.karadimov@gmail.com>
> Cc: sjm@sjm.io, eliz@gnu.org, Bozhidar Batsov <bozhidar.batsov@gmail.com>
> 
> > Emacs on Windows can only monitor and kill its immediate subprocesses,
> > it cannot monitor, let alone kill, any of their descendant processes,> because it has no idea about them.  And the OS doesn't automatically> kill all the processes in the subprocess tree, and there's no way to> send a signal to them all, as on Posix platforms.  If killing the> immediate child process doesn't cause some of its children to exit or> abort, then those grandchildren will be left orphaned.
> Windows NT does have the concept of parent processes, but an API call
> wasn't exposed in win32 until XP. I wrote a small patch that uses that
> and kills all child processes (as long as pid<0). I did some sanity
> testing and it works.

Thanks, but I don't think we can use this code safely: there's a race
condition here between the time the snapshot is taken and the time the
process in the snapshot is killed.  During this time window, however
small, a process ID can be reused for a completely different process.
Killing an unrelated process is a no-no.

Moreover, AFAIU the code takes multiple independent snapshots of the
process tree, which are not guaranteed to be consistent between them
on a live system which resuses process IDs as fast as Windows does.

The only safe way on Windows to make sure a process ID is not reused
is to keep a handle open on the process.  But such a strategy would
require some kind of notification to Emacs from its subprocesses when
they launch their subprocesses.  If you (or someone else) know how to
set this up, we could indeed resolve this problem.  Otherwise, I'm
afraid we will need to live with this some more.

> - there is no OS detection - this will get ugly on Windows 9x and
> NT4/5.0

This is not a problem: we already call these functions in Emacs, and
have the necessary machinery to detect whether the API is available.
Take a look at create_toolhelp32_snapshot in w32.c, and its callers.

> - performance: 3 API calls are made for each descendant
> process. This can be reduced to a total 3 calls (regardless of the
> child process count)

I doubt that this should count as a problem, since we are talking
about extreme cases anyway.

The only real problem is the one I mention above.

Thanks for working on this.

bug#15983: 24.3; Emacs Not Killing Child Process

[Joan Karadimov]

[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]

Thanks for the feedback!

Taking multiple independent snapshots is not something I intended to leave
this way.This is what I was referring to in the "performace" part of the
issues.

Back to the main issue - I wasn't aware that pid's get reused so rapidly on
Windows.

As for the implementation - your idea sounds great but I have no idea how to
put it together.

I am able to come up with some other stuff that use snapshots and do not
kill
unrelated processes. However, they skip any processes that are spawned after
the sys_kill subroutine is called.

Now I am starting to think in another direction. Would something like:
  system("taskkill /PID XXXX /T")
... be an acceptable solution?

On Thu, Dec 19, 2013 at 7:24 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Thu, 19 Dec 2013 17:44:37 +0200
> > From: Joan Karadimov <joan.karadimov@gmail.com>
> > Cc: sjm@sjm.io, eliz@gnu.org, Bozhidar Batsov <bozhidar.batsov@gmail.com
> >
> >
> > > Emacs on Windows can only monitor and kill its immediate subprocesses,
> > > it cannot monitor, let alone kill, any of their descendant processes,>
> because it has no idea about them.  And the OS doesn't automatically> kill
> all the processes in the subprocess tree, and there's no way to> send a
> signal to them all, as on Posix platforms.  If killing the> immediate child
> process doesn't cause some of its children to exit or> abort, then those
> grandchildren will be left orphaned.
> > Windows NT does have the concept of parent processes, but an API call
> > wasn't exposed in win32 until XP. I wrote a small patch that uses that
> > and kills all child processes (as long as pid<0). I did some sanity
> > testing and it works.
>
> Thanks, but I don't think we can use this code safely: there's a race
> condition here between the time the snapshot is taken and the time the
> process in the snapshot is killed.  During this time window, however
> small, a process ID can be reused for a completely different process.
> Killing an unrelated process is a no-no.
>
> Moreover, AFAIU the code takes multiple independent snapshots of the
> process tree, which are not guaranteed to be consistent between them
> on a live system which resuses process IDs as fast as Windows does.
>
> The only safe way on Windows to make sure a process ID is not reused
> is to keep a handle open on the process.  But such a strategy would
> require some kind of notification to Emacs from its subprocesses when
> they launch their subprocesses.  If you (or someone else) know how to
> set this up, we could indeed resolve this problem.  Otherwise, I'm
> afraid we will need to live with this some more.
>
> > - there is no OS detection - this will get ugly on Windows 9x and
> > NT4/5.0
>
> This is not a problem: we already call these functions in Emacs, and
> have the necessary machinery to detect whether the API is available.
> Take a look at create_toolhelp32_snapshot in w32.c, and its callers.
>
> > - performance: 3 API calls are made for each descendant
> > process. This can be reduced to a total 3 calls (regardless of the
> > child process count)
>
> I doubt that this should count as a problem, since we are talking
> about extreme cases anyway.
>
> The only real problem is the one I mention above.
>
> Thanks for working on this.
>

[-- Attachment #2: Type: text/html, Size: 4398 bytes --]

bug#15983: 24.3; Emacs Not Killing Child Process

[Eli Zaretskii]

> Date: Sat, 21 Dec 2013 00:28:02 +0200
> From: Joan Karadimov <joan.karadimov@gmail.com>
> Cc: Simon Morgan <sjm@sjm.io>, Bozhidar Batsov <bozhidar.batsov@gmail.com>
> 
> Back to the main issue - I wasn't aware that pid's get reused so rapidly on
> Windows.

Windows uses the same pool of numbers for process and thread IDs, so
the numbers are reused very quickly.

> As for the implementation - your idea sounds great but I have no idea how to
> put it together.

Neither do I.

> I am able to come up with some other stuff that use snapshots and do not
> kill
> unrelated processes. However, they skip any processes that are spawned after
> the sys_kill subroutine is called.

This might be "good enough" -- we err on the safe side, and only leave
some subprocesses not killed in rare situations.  Does this strategy
solve the problem which started this bug report?  If so, please tell
the main ideas of how you intend to avoid killing unrelated processes.

> Now I am starting to think in another direction. Would something like:
>   system("taskkill /PID XXXX /T")
> ... be an acceptable solution?

Unfortunately, taskkill is not available widely enough.  E.g., on my
XP Home edition, it is not available, and I believe it is not there on
systems older than XP, which we still support.

Thanks.

bug#15983: 24.3; Emacs Not Killing Child Process

[Joan Karadimov]

[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]

> Unfortunately, taskkill is not available widely enough.  E.g., on my
> XP Home edition, it is not available, and I believe it is not there on
> systems older than XP, which we still support.

I am aware that 'taskkill' is not present on windowses (is that a word?)
older
than XP. This makes it no worse than 'CreateToolhelp32Snapshot'. But I din't
know of its absence from home editions. My first thought was to seek
inspiration
in the Wine project:
  (https://github.com/mirrors/wine/blob/master/programs/taskkill/taskkill.c)
... only to find:
  WINE_FIXME("/T not supported\n");
... so this seems like a dead end.

> This might be "good enough" -- we err on the safe side, and only leave
> some subprocesses not killed in rare situations.  Does this strategy
> solve the problem which started this bug report?  If so, please tell
> the main ideas of how you intend to avoid killing unrelated processes.

Here is some pseudo-code for it...

# This returns [a subset] of the edges in the process tree
def get-process-tree:
  1. let process-snapshot be the current process snapshot
  2. let process-tree be an empty list
  3. for parent-pid, child-pid in process-snapshot:
       if process-start-time(child-pid) < process-start-time(parent-pid):
         add(process-tree, (parent-pid . child-pid))

def kill-process-tree(root-process):
  1. open a process handle to the root-process (I am guessing that Emacs
already
     keeps a handle to all process it spawned so this step might be
unnecessary)
  2. let initial-process-tree be the return value of get-process-tree
  3. let inital-kill-list be the edges from initial-process-tree that are
     reachable from the root-process
  4. open process handles to every child-pid from inital-kill-list
  5. let revised-process-tree be the return value of get-process-tree
  6. let revised-kill-list be the intersection of inital-kill-list and
     revised-process-tree
  7. kill and close handles to child processes in final-kill-list
  8. close any remaining handles from step (4.)

There are a few non-obvious things here that need a skeptical reading.

The most arcane is that step (6.) in kill-process-tree really returns what
we
need to kill.

Other potential issue is the non-atomicity of step (4.) and the possibility
of
grabbing a handle to a process that wasn't in initial-process-tree. I claim
that
this is not an issue, because those will not end up in revised-kill-list.

Both, of course, I cannot prove formally. But so far I have been unable to
find
counterexamples.

[-- Attachment #2: Type: text/html, Size: 3298 bytes --]

bug#15983: 24.3; Emacs Not Killing Child Process

[Eli Zaretskii]

> Date: Sat, 21 Dec 2013 18:22:06 +0200
> From: Joan Karadimov <joan.karadimov@gmail.com>
> Cc: 15983@debbugs.gnu.org, Simon Morgan <sjm@sjm.io>, 
> 	Bozhidar Batsov <bozhidar.batsov@gmail.com>
> 
> > Unfortunately, taskkill is not available widely enough.  E.g., on my
> > XP Home edition, it is not available, and I believe it is not there on
> > systems older than XP, which we still support.
> 
> I am aware that 'taskkill' is not present on windowses (is that a word?)
> older than XP. This makes it no worse than 'CreateToolhelp32Snapshot'.

No, the toolhelp functions are available on Windows 2000 and even on
Windows 98.  They are unavailable only on NT 4.0.

> > This might be "good enough" -- we err on the safe side, and only leave
> > some subprocesses not killed in rare situations.  Does this strategy
> > solve the problem which started this bug report?

You didn't answer that question, but I assume the answer is YES.

>  If so, please tell
> > the main ideas of how you intend to avoid killing unrelated processes.
> 
> Here is some pseudo-code for it...
> 
> # This returns [a subset] of the edges in the process tree
> def get-process-tree:
>   1. let process-snapshot be the current process snapshot
>   2. let process-tree be an empty list
>   3. for parent-pid, child-pid in process-snapshot:
>        if process-start-time(child-pid) < process-start-time(parent-pid):
>          add(process-tree, (parent-pid . child-pid))

I think it would be better to also require that process-start-time is
before the time kill-process-tree is called.  This might miss some
children, if they happen to be spawned right after the call, but it is
safer.

Also, didn't you mean ">" in the above inequality?  A child process
cannot be born before its parent, right?  Or am I missing something?

> 
> def kill-process-tree(root-process):
>   1. open a process handle to the root-process (I am guessing that Emacs
> already
>      keeps a handle to all process it spawned so this step might be
> unnecessary)

Yes, Emacs already keeps a handle on each of its immediate child
processes.  That's how we know that those children exit.

> Other potential issue is the non-atomicity of step (4.) and the
> possibility of grabbing a handle to a process that wasn't in
> initial-process-tree. I claim that this is not an issue, because
> those will not end up in revised-kill-list.
> 
> Both, of course, I cannot prove formally. But so far I have been
> unable to find counterexamples.

The only thing that we should worry about is not to accidentally kill
unrelated processes.  Everything else is no worse than what we have
now.

bug#15983: 24.3; Emacs Not Killing Child Process

[Joan Karadimov]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

>
> > I am aware that 'taskkill' is not present on windowses (is that a word?)
> > older than XP. This makes it no worse than 'CreateToolhelp32Snapshot'.
>
> No, the toolhelp functions are available on Windows 2000 and even on
> Windows 98.  They are unavailable only on NT 4.0.
>
MSDN states that the "Minimum supported client" is XP. I guess 2000 is
counted with the server ones and 9x is not even considered.


> > > This might be "good enough" -- we err on the safe side, and only leave
> > > some subprocesses not killed in rare situations.  Does this strategy
> > > solve the problem which started this bug report?
>
> You didn't answer that question, but I assume the answer is YES.
>
It should fix the problem, yes. And it should be safe


> I think it would be better to also require that process-start-time is
> before the time kill-process-tree is called.  This might miss some
> children, if they happen to be spawned right after the call, but it is
> safer.
>
This should already be reflected in the requirement that all processes that
are killed were already in the initial-process-tree (the first snapshot).
But there is no harm in being more explicit about it in the code.

Also, didn't you mean ">" in the above inequality?  A child process
> cannot be born before its parent, right?  Or am I missing something?
>
Yes, of course. You are not missing anything.


> The only thing that we should worry about is not to accidentally kill
> unrelated processes.  Everything else is no worse than what we have
> now.
>
I'll start working on some code that I can show, then.

[-- Attachment #2: Type: text/html, Size: 2924 bytes --]

bug#15983: 24.3; Emacs Not Killing Child Process

[Eli Zaretskii]

> Date: Sun, 22 Dec 2013 04:03:12 +0200
> From: Joan Karadimov <joan.karadimov@gmail.com>
> Cc: 15983@debbugs.gnu.org, Simon Morgan <sjm@sjm.io>, 
> 	Bozhidar Batsov <bozhidar.batsov@gmail.com>
> 
> > > I am aware that 'taskkill' is not present on windowses (is that a word?)
> > > older than XP. This makes it no worse than 'CreateToolhelp32Snapshot'.
> >
> > No, the toolhelp functions are available on Windows 2000 and even on
> > Windows 98.  They are unavailable only on NT 4.0.
> >
> MSDN states that the "Minimum supported client" is XP.

MSDN lies.  They do that in a lot of API functions, to pretend that
older versions don't exist (because their support has ended).

> I guess 2000 is counted with the server ones

No, it's not.

> and 9x is not even considered.

Right.

> > > > This might be "good enough" -- we err on the safe side, and only leave
> > > > some subprocesses not killed in rare situations.  Does this strategy
> > > > solve the problem which started this bug report?
> >
> > You didn't answer that question, but I assume the answer is YES.
> >
> It should fix the problem, yes.

Well, I'd prefer a test, to be sure.

> > I think it would be better to also require that process-start-time is
> > before the time kill-process-tree is called.  This might miss some
> > children, if they happen to be spawned right after the call, but it is
> > safer.
> >
> This should already be reflected in the requirement that all processes that
> are killed were already in the initial-process-tree (the first snapshot).

They could have been started before that.  E.g., imagine that one of
the children exited or died on its own, and its PID was reused, before
kill-process was called.

> I'll start working on some code that I can show, then.

Thank you.

Btw, if the patch is going to be substantial, we will need legal
paperwork from you, before we can accept the code.

bug#15983: 24.3; Emacs Not Killing Child Process

[Joan Karadimov]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

>
> > > > > This might be "good enough" -- we err on the safe side, and only
> leave
> > > > > some subprocesses not killed in rare situations.  Does this
> strategy
> > > > > solve the problem which started this bug report?
> > >
> > > You didn't answer that question, but I assume the answer is YES.
> > >
> > It should fix the problem, yes.
>
> Well, I'd prefer a test, to be sure.
>
> Next few days I will be spending less time on this. I'll have something
testable
a few days after Christmas.

Btw, if the patch is going to be substantial, we will need legal
> paperwork from you, before we can accept the code.
>
Yes, I know. This is forming to be larger than I initially thought. Maybe
once
I am done we can evaluate just how big the change set is and I'll do the
paperwork if needed.

[-- Attachment #2: Type: text/html, Size: 1500 bytes --]