From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#12829: 24.3.50; emacs_abort () called from w32proc.c:1128 Date: Sat, 10 Nov 2012 16:56:47 +0200 Message-ID: <83haoxy09c.fsf@gnu.org> References: <509D529F.9090106@optusnet.com.au> <83wqxuy3bz.fsf@gnu.org> <509D64A4.9070608@optusnet.com.au> <83sj8hyiap.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1352559478 9419 80.91.229.3 (10 Nov 2012 14:57:58 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 10 Nov 2012 14:57:58 +0000 (UTC) Cc: eggert@cs.ucla.edu, 12829@debbugs.gnu.org To: stephen_powell@optusnet.com.au Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Nov 10 15:58:07 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TXCVa-0006eL-CZ for geb-bug-gnu-emacs@m.gmane.org; Sat, 10 Nov 2012 15:58:06 +0100 Original-Received: from localhost ([::1]:49610 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXCVR-0001DJ-68 for geb-bug-gnu-emacs@m.gmane.org; Sat, 10 Nov 2012 09:57:57 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:34827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXCVM-0001Cx-3c for bug-gnu-emacs@gnu.org; Sat, 10 Nov 2012 09:57:55 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TXCVJ-00037t-1q for bug-gnu-emacs@gnu.org; Sat, 10 Nov 2012 09:57:52 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:49369) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXCVI-00037p-UL for bug-gnu-emacs@gnu.org; Sat, 10 Nov 2012 09:57:48 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1TXCVV-0007xf-MP for bug-gnu-emacs@gnu.org; Sat, 10 Nov 2012 09:58:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 10 Nov 2012 14:58:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 12829 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 12829-submit@debbugs.gnu.org id=B12829.135255943030538 (code B ref 12829); Sat, 10 Nov 2012 14:58:01 +0000 Original-Received: (at 12829) by debbugs.gnu.org; 10 Nov 2012 14:57:10 +0000 Original-Received: from localhost ([127.0.0.1]:59620 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TXCUf-0007wU-Ju for submit@debbugs.gnu.org; Sat, 10 Nov 2012 09:57:10 -0500 Original-Received: from mtaout20.012.net.il ([80.179.55.166]:37737) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TXCUa-0007wK-GI for 12829@debbugs.gnu.org; Sat, 10 Nov 2012 09:57:06 -0500 Original-Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0MDA00B001CIF500@a-mtaout20.012.net.il> for 12829@debbugs.gnu.org; Sat, 10 Nov 2012 16:56:38 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MDA00BQ81IDCC80@a-mtaout20.012.net.il>; Sat, 10 Nov 2012 16:56:38 +0200 (IST) In-reply-to: <83sj8hyiap.fsf@gnu.org> X-012-Sender: halo1@inter.net.il X-Spam-Score: 0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-Spam-Score: -1.2 (-) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:66726 Archived-At: > Date: Sat, 10 Nov 2012 10:27:10 +0200 > From: Eli Zaretskii > Cc: 12829@debbugs.gnu.org > > Thanks, that figures. Basically, the modified code that handles > demise of child processes is incompatible with the emulated 'wait' > function, because it does not support waiting for a process by its > PID. I think we will have to rewrite 'sys_wait' to emulate 'waitpid', > although I'll try first to come up with a simpler band-aid. I'd still like to have the details as requested here (with the current trunk code): > Just so my understanding of the exact scenario is better, could you > please add "bt 10" to the commands of breakpoint 4, the one set in > process_status_retrieved, and again post the full transcript of the > GDB session leading to the crash? However in the meantime, I came up with the changes below, which seem to work in my limited testing, and should fix the fundamental problem that caused your crashes. Could you please try these changes for a while, and see if they give good results? (You will have to re-run configure.bat before compiling.) If they do, I will then commit this to the trunk. TIA. === modified file 'nt/config.nt' --- nt/config.nt 2012-11-05 14:30:32 +0000 +++ nt/config.nt 2012-11-10 12:02:26 +0000 @@ -959,7 +959,7 @@ along with GNU Emacs. If not, see that is POSIX.1 compatible. */ -#undef HAVE_SYS_WAIT_H +#define HAVE_SYS_WAIT_H 1 /* Define to 1 if you have the header file. */ #undef HAVE_TERM_H === modified file 'nt/inc/ms-w32.h' --- nt/inc/ms-w32.h 2012-10-19 19:25:18 +0000 +++ nt/inc/ms-w32.h 2012-11-10 12:06:16 +0000 @@ -185,15 +185,12 @@ extern char *getenv (); /* Subprocess calls that are emulated. */ #define spawnve sys_spawnve -#define wait sys_wait #define kill sys_kill #define signal sys_signal /* Internal signals. */ #define emacs_raise(sig) emacs_abort() -extern int sys_wait (int *); - /* termcap.c calls that are emulated. */ #define tputs sys_tputs #define tgetstr sys_tgetstr === added file 'nt/inc/sys/wait.h' --- nt/inc/sys/wait.h 1970-01-01 00:00:00 +0000 +++ nt/inc/sys/wait.h 2012-11-10 12:05:45 +0000 @@ -0,0 +1,33 @@ +/* A limited emulation of sys/wait.h on Posix systems. + +Copyright (C) 2012 Free Software Foundation, Inc. + +This file is part of GNU Emacs. + +GNU Emacs is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +GNU Emacs is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GNU Emacs. If not, see . */ + +#ifndef INC_SYS_WAIT_H_ +#define INC_SYS_WAIT_H_ + +#define WNOHANG 1 +#define WUNTRACED 2 +#define WSTOPPED 2 /* same as WUNTRACED */ +#define WEXITED 4 +#define WCONTINUED 8 + +/* The various WIF* macros are defined in src/syswait.h. */ + +extern pid_t waitpid (pid_t, int *, int); + +#endif /* INC_SYS_WAIT_H_ */ === modified file 'src/sysdep.c' --- src/sysdep.c 2012-11-05 03:18:32 +0000 +++ src/sysdep.c 2012-11-10 13:58:31 +0000 @@ -290,7 +290,7 @@ wait_for_termination_1 (pid_t pid, int i while (1) { #ifdef WINDOWSNT - wait (0); + waitpid (pid, NULL, 0); break; #else /* not WINDOWSNT */ int status; === modified file 'src/w32proc.c' --- src/w32proc.c 2012-11-05 03:18:32 +0000 +++ src/w32proc.c 2012-11-10 14:04:54 +0000 @@ -775,7 +775,6 @@ alarm (int seconds) /* Child process management list. */ int child_proc_count = 0; child_process child_procs[ MAX_CHILDREN ]; -child_process *dead_child = NULL; static DWORD WINAPI reader_thread (void *arg); @@ -1028,9 +1027,6 @@ create_child (char *exe, char *cmdline, if (cp->pid < 0) cp->pid = -cp->pid; - /* pid must fit in a Lisp_Int */ - cp->pid = cp->pid & INTMASK; - *pPid = cp->pid; return TRUE; @@ -1106,55 +1102,110 @@ reap_subprocess (child_process *cp) delete_child (cp); } -/* Wait for any of our existing child processes to die - When it does, close its handle - Return the pid and fill in the status if non-NULL. */ +/* Wait for a child process specified by PID, or for any of our + existing child processes (if PID is nonpositive) to die. When it + does, close its handle. Return the pid of the process that died + and fill in STATUS if non-NULL. */ -int -sys_wait (int *status) +pid_t +waitpid (pid_t pid, int *status, int options) { DWORD active, retval; int nh; - int pid; child_process *cp, *cps[MAX_CHILDREN]; HANDLE wait_hnd[MAX_CHILDREN]; + DWORD timeout_ms; + int dont_wait = (options & WNOHANG) != 0; nh = 0; - if (dead_child != NULL) + /* According to Posix: + + PID = -1 means status is requested for any child process. + + PID > 0 means status is requested for a single child process + whose pid is PID. + + PID = 0 means status is requested for any child process whose + process group ID is equal to that of the calling process. But + since Windows has only a limited support for process groups (only + for console processes and only for the purposes of passing + Ctrl-BREAK signal to them), and since we have no documented way + of determining whether a given process belongs to our group, we + treat 0 as -1. + + PID < -1 means status is requested for any child process whose + process group ID is equal to the absolute value of PID. Again, + since we don't support process groups, we treat that as -1. */ + if (pid > 0) { - /* We want to wait for a specific child */ - wait_hnd[nh] = dead_child->procinfo.hProcess; - cps[nh] = dead_child; - if (!wait_hnd[nh]) emacs_abort (); - nh++; - active = 0; - goto get_result; + int our_child = 0; + + /* We are requested to wait for a specific child. */ + for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) + { + /* Some child_procs might be sockets; ignore them. Also + ignore subprocesses whose output is not yet completely + read. */ + if (CHILD_ACTIVE (cp) + && cp->procinfo.hProcess + && cp->pid == pid) + { + our_child = 1; + break; + } + } + if (our_child) + { + if (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0) + { + wait_hnd[nh] = cp->procinfo.hProcess; + cps[nh] = cp; + nh++; + } + else if (dont_wait) + { + /* PID specifies our subprocess, but its status is not + yet available. */ + return 0; + } + } + if (nh == 0) + { + /* No such child process, or nothing to wait for, so fail. */ + errno = ECHILD; + return -1; + } } else { for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) - /* some child_procs might be sockets; ignore them */ - if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess - && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) - { - wait_hnd[nh] = cp->procinfo.hProcess; - cps[nh] = cp; - nh++; - } + { + if (CHILD_ACTIVE (cp) + && cp->procinfo.hProcess + && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) + { + wait_hnd[nh] = cp->procinfo.hProcess; + cps[nh] = cp; + nh++; + } + } + if (nh == 0) + { + /* Nothing to wait on, so fail. */ + errno = ECHILD; + return -1; + } } - if (nh == 0) - { - /* Nothing to wait on, so fail */ - errno = ECHILD; - return -1; - } + if (dont_wait) + timeout_ms = 0; + else + timeout_ms = 1000; /* check for quit about once a second. */ do { - /* Check for quit about once a second. */ QUIT; - active = WaitForMultipleObjects (nh, wait_hnd, FALSE, 1000); + active = WaitForMultipleObjects (nh, wait_hnd, FALSE, timeout_ms); } while (active == WAIT_TIMEOUT); if (active == WAIT_FAILED) @@ -1184,8 +1235,10 @@ get_result: } if (retval == STILL_ACTIVE) { - /* Should never happen */ + /* Should never happen. */ DebPrint (("Wait.WaitForMultipleObjects returned an active process\n")); + if (pid > 0 && dont_wait) + return 0; errno = EINVAL; return -1; } @@ -1199,6 +1252,8 @@ get_result: else retval <<= 8; + if (pid > 0 && active != 0) + emacs_abort (); cp = cps[active]; pid = cp->pid; #ifdef FULL_DEBUG @@ -1987,9 +2042,7 @@ count_children: DebPrint (("select calling SIGCHLD handler for pid %d\n", cp->pid)); #endif - dead_child = cp; sig_handlers[SIGCHLD] (SIGCHLD); - dead_child = NULL; } } else if (fdindex[active] == -1)