Package: emacs;
Reported by: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Tue, 6 Aug 2013 17:17:02 UTC
Severity: normal
Tags: patch
Done: Paul Eggert <paul.eggert <at> verizon.net>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Paul Eggert <paul.eggert <at> verizon.net> Cc: tracker <at> debbugs.gnu.org Subject: bug#15035: closed (Fix some fd issues when running subprocesses) Date: Mon, 12 Aug 2013 07:15:03 +0000
[Message part 1 (text/plain, inline)]
Your message dated Mon, 12 Aug 2013 00:14:03 -0700 with message-id <52088B3B.3010500 <at> verizon.net> and subject line Re: Fix some fd issues when running subprocesses has caused the debbugs.gnu.org bug report #15035, regarding Fix some fd issues when running subprocesses to be marked as done. (If you believe you have received this mail in error, please contact help-debbugs <at> gnu.org.) -- 15035: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15035 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Paul Eggert <eggert <at> cs.ucla.edu> To: bug-emacs <bug-gnu-emacs <at> gnu.org> Subject: Fix some fd issues when running subprocesses Date: Tue, 06 Aug 2013 10:15:35 -0700Tags: patch Here's a proposed patch to fix some issues in call-process etc. that have been discussed on emacs-devel recently. This is a first cut; I plan to test it further before installing. I'll publishing it now since it involves changes to the Microsoft ports, and I'll CC: this to Eli as a heads-up. This is relative to trunk bzr 113721. === modified file 'src/ChangeLog' --- src/ChangeLog 2013-08-06 16:51:41 +0000 +++ src/ChangeLog 2013-08-06 17:11:59 +0000 @@ -1,3 +1,81 @@ +2013-08-06 Paul Eggert <eggert <at> cs.ucla.edu> + + Fix some fd issues when running subprocesses. + Fix bugs that can leak files or file descriptors on errors. + Don't unlink open temp files, as that's hard for users to diagnose + when things go awry (e.g., temp disk exhausted). + Don't bother to lock temp files. Check for invalid recursion. + * callproc.c (synch_process_fd): Remove. All uses removed. + (synch_process_tempfile): New var or macro. + (CALLPROC_STDOUT, CALLPROC_STDERR, CALLPROC_PIPEREAD, CALLPROC_FDS): + New constants. + (record_kill_process): New arg, the tempfile name. All callers changed. + (delete_temp_file): Now just a simple wrapper around unlink. + (call_process_kill): New arg, the call_process_fd array. + Close them all. Clear synch_process_pid. Remove the temp file, + or arrange for it to be removed. + (call_process_cleanup) [MSDOS]: Arg no longer contains file name; + that's been moved to synch_process_tempfile. Caller changed. + Do not remove the tempfile; that's now call_process_kill's + responsibility. + (call_process_cleanup) [!MSDOS]: Do not record unwind-protect for + call_process_kill; the caller now does that. + (call_process_cleanup): Do not close the process fd; that's now + call_process_kill's responsibility. + (Fcall_process): Implement via new function call_process, which + contains most of the old body of Fcall_process, but has a different API. + (call_process): New function that does not open or close filefd if + it is nonnegative. Record which fds need to be closed, and let + call_process_kill close (and remove the tempfile, on MSDOS) on error. + Signal an error if invoked recursively (could be done via a hook). + Simplify creation of the tempfile in the MSDOS case. + Don't create the output file until after checking for the executable. + Report any failure to open /dev/null. + Don't open /dev/null for writing twice; once is enough. + Don't create pipe if all output is being discarded or sent to file. + Don't worry about setting up the coding system or reading from the + pipe if all output is being discarded. + Hoist fd_error local into top level, to lessen block nesting. + Don't record deleted pid here; now done by Fcall_process_region. + (Fcall_process) [MSDOS]: Report mktemp failure immediately, + and note its success in synch_process_tempfile. + Do not leak resources when child_setup fails. + (Fcall_process) [!MSDOS && !WINDOWSNT]: Remove duplicate assignment + to child_errno. Remove unnecessary close of fd0; it's close-on-exec. + (create_temp_file): Now returns open fd, with an additional + Lisp_Object * argument to return the name. All callers changed. + Do not close the file; rewind it instead, and leave it open for + the caller. Do not lock the temp file. Unwind-protect the file + and the file-descriptor. + (Fcall_process_region): If the input is /dev/null, unwind-protect it. + If an asynchrounous process, record it here, not in call_process. + (syms_of_callproc) [MSDOS]: Initialize synch_process_tempfile. + * eval.c (set_unwind_protect): New function. + * fileio.c (write_region): New function, generalized from the + old Fwrite_region. Do not lock temp files. + (Fwrite_region): Use it. + * lisp.h (set_unwind_protect, write_region): New decls. + * process.c: Include <verify.h>. + (make_process): Mark fds as initially closed. + (deleted_pid_list): Now a list of pid-filename pairs. All uses changed. + (close_process_fd): New function. + (SUBPROCESS_STDIN, WRITE_TO_SUBPROCESS, READ_FROM_SUBPROCESS) + (SUBPROCESS_STDOUT, READ_FROM_EXEC_MONITOR, EXEC_MONITOR_OUTPUT): + New constants. Verify that their number matches PROCESS_OPEN_FDS. + (create_process, create_pty, Fmake_serial_process) + (server_accept_connection): Record which fds need to be closed, + and let deactivate_process close them. + (Fmake_network_process): Do not discard the unwind-protect + until it's safe to do so. + (deactivate_process): Close the fds opened by create_process etc. + (Fprocess_send_eof): Adjust to new way of recording open fds. + Report an error if /dev/null can't be opened, instead of aborting. + * process.h (PROCESS_OPEN_FDS): New constant. + (struct Lisp_Process): New member open_fds. + (record_kill_process, record_deleted_pid): Adjust signatures. + (record_deleted_pid): Move decl here ... + * syswait.h (record_deleted_pid): ... from here. + 2013-08-06 Dmitry Antipov <dmantipov <at> yandex.ru> * window.c (window_scroll, window_scroll_pixel_based) === modified file 'src/callproc.c' --- src/callproc.c 2013-08-04 16:56:56 +0000 +++ src/callproc.c 2013-08-06 16:35:16 +0000 @@ -68,9 +68,10 @@ /* Pattern used by call-process-region to make temp files. */ static Lisp_Object Vtemp_file_name_pattern; -/* The next two variables are valid only while record-unwind-protect - is in place during call-process for a synchronous subprocess. At - other times, their contents are irrelevant. Doing this via static +/* The next two variables are used while record-unwind-protect is in place + during call-process for a subprocess for which record_deleted_pid has + not yet been called. At other times, synch_process_pid is zero and + synch_process_tempfile's contents are irrelevant. Doing this via static C variables is more convenient than putting them into the arguments of record-unwind-protect, as they need to be updated at randomish times in the code, and Lisp cannot always store these values as @@ -80,8 +81,28 @@ /* If nonzero, a process-ID that has not been reaped. */ static pid_t synch_process_pid; -/* If nonnegative, a file descriptor that has not been closed. */ -static int synch_process_fd; +/* If a string, the name of a temp file that has not been removed. */ +#ifdef MSDOS +static Lisp_Object synch_process_tempfile; +#else +# define synch_process_tempfile make_number (0) +#endif + +/* Indexes of file descriptors that need closing on call_process_kill. */ +enum + { + /* The subsidiary process's stdout and stderr. stdin is handled + separately, in either Fcall_process_region or create_temp_file. */ + CALLPROC_STDOUT, CALLPROC_STDERR, + + /* How to read from a pipe (or substitute) from the subsidiary process. */ + CALLPROC_PIPEREAD, + + /* A bound on the number of file descriptors. */ + CALLPROC_FDS + }; + +static Lisp_Object call_process (ptrdiff_t, Lisp_Object *, int); /* Block SIGCHLD. */ @@ -107,80 +128,68 @@ reaped on receipt of the first SIGCHLD after the critical section. */ void -record_kill_process (struct Lisp_Process *p) +record_kill_process (struct Lisp_Process *p, Lisp_Object tempfile) { block_child_signal (); if (p->alive) { + record_deleted_pid (p->pid, tempfile); p->alive = 0; - record_deleted_pid (p->pid); kill (- p->pid, SIGKILL); } unblock_child_signal (); } -/* Clean up when exiting call_process_cleanup. */ - -static void -call_process_kill (void) -{ - if (synch_process_fd >= 0) - emacs_close (synch_process_fd); +/* Clean up files, file descriptors and processes created by Fcall_process. */ + +static void +delete_temp_file (Lisp_Object name) +{ + unlink (SSDATA (name)); +} + +static void +call_process_kill (void *ptr) +{ + int *callproc_fd = ptr; + int i; + for (i = 0; i < CALLPROC_FDS; i++) + if (0 <= callproc_fd[i]) + emacs_close (callproc_fd[i]); if (synch_process_pid) { struct Lisp_Process proc; proc.alive = 1; proc.pid = synch_process_pid; - record_kill_process (&proc); + record_kill_process (&proc, synch_process_tempfile); + synch_process_pid = 0; } + else if (STRINGP (synch_process_tempfile)) + delete_temp_file (synch_process_tempfile); } -/* Clean up when exiting Fcall_process. - On MSDOS, delete the temporary file on any kind of termination. - On Unix, kill the process and any children on termination by signal. */ +/* Clean up when exiting Fcall_process: restore the buffer, and + kill the subsidiary process group if the process still exists. */ static void -call_process_cleanup (Lisp_Object arg) +call_process_cleanup (Lisp_Object buffer) { -#ifdef MSDOS - Lisp_Object buffer = Fcar (arg); - Lisp_Object file = Fcdr (arg); -#else - Lisp_Object buffer = arg; -#endif - Fset_buffer (buffer); -#ifndef MSDOS - /* If the process still exists, kill its process group. */ if (synch_process_pid) { - ptrdiff_t count = SPECPDL_INDEX (); kill (-synch_process_pid, SIGINT); - record_unwind_protect_void (call_process_kill); message1 ("Waiting for process to die...(type C-g again to kill it instantly)"); immediate_quit = 1; QUIT; wait_for_termination (synch_process_pid, 0, 1); synch_process_pid = 0; immediate_quit = 0; - specpdl_ptr = specpdl + count; /* Discard the unwind protect. */ message1 ("Waiting for process to die...done"); } -#endif - - if (synch_process_fd >= 0) - emacs_close (synch_process_fd); - -#ifdef MSDOS - /* FILE is "" when we didn't actually create a temporary file in - call-process. */ - if (!(strcmp (SDATA (file), NULL_DEVICE) == 0 || SREF (file, 0) == '\0')) - unlink (SDATA (file)); -#endif } #ifdef DOS_NT @@ -218,10 +227,42 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS) */) (ptrdiff_t nargs, Lisp_Object *args) { - Lisp_Object infile, buffer, current_dir, path; + Lisp_Object infile, encoded_infile; + int filefd; + struct gcpro gcpro1; + ptrdiff_t count = SPECPDL_INDEX (); + + if (nargs >= 2 && ! NILP (args[1])) + { + infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); + CHECK_STRING (infile); + } + else + infile = build_string (NULL_DEVICE); + + GCPRO1 (infile); + encoded_infile = STRING_MULTIBYTE (infile) ? ENCODE_FILE (infile) : infile; + + filefd = emacs_open (SSDATA (encoded_infile), O_RDONLY, 0); + if (filefd < 0) + report_file_error ("Opening process input file", infile); + record_unwind_protect_int (close_file_unwind, filefd); + UNGCPRO; + return unbind_to (count, call_process (nargs, args, filefd)); +} + +/* Like Fcall_process (NARGS, ARGS), except use FILEFD as the input file. + At entry, the specpdl stack top entry must be close_file_unwind (FILEFD). */ + +static Lisp_Object +call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd) +{ + Lisp_Object buffer, current_dir, path; bool display_p; - int fd0, fd1, filefd; + int fd0; + int callproc_fd[CALLPROC_FDS]; int status; + ptrdiff_t i; ptrdiff_t count = SPECPDL_INDEX (); USE_SAFE_ALLOCA; @@ -231,19 +272,21 @@ Lisp_Object error_file; Lisp_Object output_file = Qnil; #ifdef MSDOS /* Demacs 1.1.1 91/10/16 HIRANO Satoshi */ - char *outf, *tempfile = NULL; - int outfilefd; + char *tempfile = NULL; int pid; #else pid_t pid; #endif int child_errno; - int fd_output = -1; + int fd_output, fd_error; struct coding_system process_coding; /* coding-system of process output */ struct coding_system argument_coding; /* coding-system of arguments */ /* Set to the return value of Ffind_operation_coding_system. */ Lisp_Object coding_systems; - bool output_to_buffer = 1; + bool discard_output; + + if (synch_process_pid) + error ("call-process invoked recursively"); /* Qt denotes that Ffind_operation_coding_system is not yet called. */ coding_systems = Qt; @@ -262,7 +305,6 @@ /* Decide the coding-system for giving arguments. */ { Lisp_Object val, *args2; - ptrdiff_t i; /* If arguments are supplied, we may have to encode them. */ if (nargs >= 5) @@ -301,24 +343,16 @@ } } - if (nargs >= 2 && ! NILP (args[1])) - { - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); - CHECK_STRING (infile); - } + if (nargs < 3) + buffer = Qnil; else - infile = build_string (NULL_DEVICE); - - if (nargs >= 3) { buffer = args[2]; /* If BUFFER is a list, its meaning is (BUFFER-FOR-STDOUT FILE-FOR-STDERR), unless the first element is :file, in which case see the next paragraph. */ - if (CONSP (buffer) - && (! SYMBOLP (XCAR (buffer)) - || strcmp (SSDATA (SYMBOL_NAME (XCAR (buffer))), ":file"))) + if (CONSP (buffer) && !EQ (XCAR (buffer), QCfile)) { if (CONSP (XCDR (buffer))) { @@ -335,9 +369,7 @@ } /* If the buffer is (still) a list, it might be a (:file "file") spec. */ - if (CONSP (buffer) - && SYMBOLP (XCAR (buffer)) - && ! strcmp (SSDATA (SYMBOL_NAME (XCAR (buffer))), ":file")) + if (CONSP (buffer) && EQ (XCAR (buffer), QCfile)) { output_file = Fexpand_file_name (XCAR (XCDR (buffer)), BVAR (current_buffer, directory)); @@ -345,9 +377,7 @@ buffer = Qnil; } - if (!(EQ (buffer, Qnil) - || EQ (buffer, Qt) - || INTEGERP (buffer))) + if (! (NILP (buffer) || EQ (buffer, Qt) || INTEGERP (buffer))) { Lisp_Object spec_buffer; spec_buffer = buffer; @@ -358,8 +388,6 @@ CHECK_BUFFER (buffer); } } - else - buffer = Qnil; /* Make sure that the child will be able to chdir to the current buffer's current directory, or its unhandled equivalent. We @@ -372,11 +400,11 @@ protected by the caller, so all we really have to worry about is buffer. */ { - struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5; + struct gcpro gcpro1, gcpro2, gcpro3, gcpro4; current_dir = BVAR (current_buffer, directory); - GCPRO5 (infile, buffer, current_dir, error_file, output_file); + GCPRO4 (buffer, current_dir, error_file, output_file); current_dir = Funhandled_file_name_directory (current_dir); if (NILP (current_dir)) @@ -390,8 +418,6 @@ report_file_error ("Setting current directory", BVAR (current_buffer, directory)); - if (STRING_MULTIBYTE (infile)) - infile = ENCODE_FILE (infile); if (STRING_MULTIBYTE (current_dir)) current_dir = ENCODE_FILE (current_dir); if (STRINGP (error_file) && STRING_MULTIBYTE (error_file)) @@ -403,44 +429,23 @@ display_p = INTERACTIVE && nargs >= 4 && !NILP (args[3]); - filefd = emacs_open (SSDATA (infile), O_RDONLY, 0); - if (filefd < 0) - { - int open_errno = errno; - report_file_errno ("Opening process input file", DECODE_FILE (infile), - open_errno); - } - - if (STRINGP (output_file)) - { - fd_output = emacs_open (SSDATA (output_file), - O_WRONLY | O_CREAT | O_TRUNC | O_TEXT, - default_output_mode); - if (fd_output < 0) - { - int open_errno = errno; - output_file = DECODE_FILE (output_file); - report_file_errno ("Opening process output file", - output_file, open_errno); - } - if (STRINGP (error_file) || NILP (error_file)) - output_to_buffer = 0; - } + for (i = 0; i < CALLPROC_FDS; i++) + callproc_fd[i] = -1; +#ifdef MSDOS + synch_process_tempfile = make_number (0); +#endif + record_unwind_protect_ptr (call_process_kill, callproc_fd); /* Search for program; barf if not found. */ { - struct gcpro gcpro1, gcpro2, gcpro3, gcpro4; + struct gcpro gcpro1, gcpro2, gcpro3; int ok; - GCPRO4 (infile, buffer, current_dir, error_file); + GCPRO3 (buffer, current_dir, error_file); ok = openp (Vexec_path, args[0], Vexec_suffixes, &path, make_number (X_OK)); UNGCPRO; if (ok < 0) - { - int openp_errno = errno; - emacs_close (filefd); - report_file_errno ("Searching for program", args[0], openp_errno); - } + report_file_error ("Searching for program", args[0]); } /* If program file name starts with /: for quoting a magic name, @@ -452,9 +457,9 @@ new_argv = SAFE_ALLOCA ((nargs > 4 ? nargs - 2 : 2) * sizeof *new_argv); { - struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5; + struct gcpro gcpro1, gcpro2, gcpro3, gcpro4; - GCPRO5 (infile, buffer, current_dir, path, error_file); + GCPRO4 (buffer, current_dir, path, error_file); if (nargs > 4) { ptrdiff_t i; @@ -479,254 +484,213 @@ UNGCPRO; } -#ifdef MSDOS /* MW, July 1993 */ + discard_output = INTEGERP (buffer) || (NILP (buffer) && NILP (output_file)); - /* If we're redirecting STDOUT to a file, that file is already open - on fd_output. */ - if (fd_output < 0) +#ifdef MSDOS + if (! discard_output && ! STRINGP (output_file)) { - if ((outf = egetenv ("TMPDIR"))) - strcpy (tempfile = alloca (strlen (outf) + 20), outf); - else - { - tempfile = alloca (20); - *tempfile = '\0'; - } + char const *tmpdir = egetenv ("TMPDIR"); + char const *outf = tmpdir ? tmpdir : ""; + tempfile = alloca (strlen (outf) + 20); + strcpy (tempfile, outf); dostounix_filename (tempfile, 0); if (*tempfile == '\0' || tempfile[strlen (tempfile) - 1] != '/') strcat (tempfile, "/"); strcat (tempfile, "detmp.XXX"); mktemp (tempfile); - outfilefd = emacs_open (tempfile, O_WRONLY | O_CREAT | O_TRUNC, - S_IREAD | S_IWRITE); - if (outfilefd < 0) + if (!*tempfile) + report_file_error ("Opening process output file", Qnil); + output_file = build_string (tempfile); + synch_process_tempfile = output_file; + } +#endif + + if (discard_output) + { + fd_output = emacs_open (NULL_DEVICE, O_WRONLY, 0); + if (fd_output < 0) + report_file_error ("Opening null device", Qnil); + } + else if (STRINGP (output_file)) + { + fd_output = emacs_open (SSDATA (output_file), + O_WRONLY | O_CREAT | O_TRUNC | O_TEXT, + default_output_mode); + if (fd_output < 0) { int open_errno = errno; - emacs_close (filefd); + output_file = DECODE_FILE (output_file); report_file_errno ("Opening process output file", - build_string (tempfile), open_errno); + output_file, open_errno); } } else - outfilefd = fd_output; - fd0 = filefd; - fd1 = outfilefd; -#endif /* MSDOS */ - - if (INTEGERP (buffer)) - { - fd0 = -1; - fd1 = emacs_open (NULL_DEVICE, O_WRONLY, 0); - } - else - { -#ifndef MSDOS + { int fd[2]; if (emacs_pipe (fd) != 0) - { - int pipe_errno = errno; - emacs_close (filefd); - report_file_errno ("Creating process pipe", Qnil, pipe_errno); - } - fd0 = fd[0]; - fd1 = fd[1]; -#endif + report_file_error ("Creating process pipe", Qnil); + callproc_fd[CALLPROC_PIPEREAD] = fd[0]; + fd_output = fd[1]; } - - { - int fd_error = fd1; - - if (fd_output >= 0) - fd1 = fd_output; - - if (NILP (error_file)) - fd_error = emacs_open (NULL_DEVICE, O_WRONLY, 0); - else if (STRINGP (error_file)) - fd_error = emacs_open (SSDATA (error_file), + callproc_fd[CALLPROC_STDOUT] = fd_output; + + fd_error = fd_output; + + if (STRINGP (error_file) || (NILP (error_file) && !discard_output)) + { + fd_error = emacs_open ((STRINGP (error_file) + ? SSDATA (error_file) + : NULL_DEVICE), O_WRONLY | O_CREAT | O_TRUNC | O_TEXT, default_output_mode); - - if (fd_error < 0) - { - int open_errno = errno; - emacs_close (filefd); - if (fd0 != filefd) - emacs_close (fd0); - if (fd1 >= 0) - emacs_close (fd1); -#ifdef MSDOS - unlink (tempfile); -#endif - if (NILP (error_file)) - error_file = build_string (NULL_DEVICE); - else if (STRINGP (error_file)) - error_file = DECODE_FILE (error_file); - report_file_errno ("Cannot redirect stderr", error_file, open_errno); - } + if (fd_error < 0) + { + int open_errno = errno; + report_file_errno ("Cannot redirect stderr", + (STRINGP (error_file) + ? DECODE_FILE (error_file) + : build_string (NULL_DEVICE)), + open_errno); + } + callproc_fd[CALLPROC_STDERR] = fd_error; + } #ifdef MSDOS /* MW, July 1993 */ - /* Note that on MSDOS `child_setup' actually returns the child process - exit status, not its PID, so assign it to status below. */ - pid = child_setup (filefd, outfilefd, fd_error, new_argv, 0, current_dir); - child_errno = errno; - - emacs_close (outfilefd); - if (fd_error != outfilefd) - emacs_close (fd_error); - if (pid < 0) - { - synchronize_system_messages_locale (); - return - code_convert_string_norecord (build_string (strerror (child_errno)), - Vlocale_coding_system, 0); - } - status = pid; - fd1 = -1; /* No harm in closing that one! */ - if (tempfile) - { - /* Since CRLF is converted to LF within `decode_coding', we - can always open a file with binary mode. */ - fd0 = emacs_open (tempfile, O_RDONLY | O_BINARY, 0); - if (fd0 < 0) - { - int open_errno = errno; - unlink (tempfile); - emacs_close (filefd); - report_file_errno ("Cannot re-open temporary file", - build_string (tempfile), open_errno); - } - } - else - fd0 = -1; /* We are not going to read from tempfile. */ -#endif /* MSDOS */ - - /* Do the unwind-protect now, even though the pid is not known, so - that no storage allocation is done in the critical section. - The actual PID will be filled in during the critical section. */ - synch_process_pid = 0; - synch_process_fd = fd0; - -#ifdef MSDOS - /* MSDOS needs different cleanup information. */ - record_unwind_protect (call_process_cleanup, - Fcons (Fcurrent_buffer (), - build_string (tempfile ? tempfile : ""))); -#else - record_unwind_protect (call_process_cleanup, Fcurrent_buffer ()); - - block_input (); - block_child_signal (); - -#ifdef WINDOWSNT - pid = child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir); - /* We need to record the input file of this child, for when we are - called from call-process-region to create an async subprocess. - That's because call-process-region's unwind procedure will - attempt to delete the temporary input file, which will fail - because that file is still in use. Recording it with the child - will allow us to delete the file when the subprocess exits. - The second part of this is in delete_temp_file, q.v. */ - if (pid > 0 && INTEGERP (buffer) && nargs >= 2 && !NILP (args[1])) - record_infile (pid, xstrdup (SSDATA (infile))); -#else /* not WINDOWSNT */ - - /* vfork, and prevent local vars from being clobbered by the vfork. */ + /* Note that on MSDOS `child_setup' actually returns the child process + exit status, not its PID, so assign it to status below. */ + pid = child_setup (filefd, fd_output, fd_error, new_argv, 0, current_dir); + + if (pid < 0) { - Lisp_Object volatile buffer_volatile = buffer; - Lisp_Object volatile coding_systems_volatile = coding_systems; - Lisp_Object volatile current_dir_volatile = current_dir; - bool volatile display_p_volatile = display_p; - bool volatile output_to_buffer_volatile = output_to_buffer; - bool volatile sa_must_free_volatile = sa_must_free; - int volatile fd1_volatile = fd1; - int volatile fd_error_volatile = fd_error; - int volatile fd_output_volatile = fd_output; - int volatile filefd_volatile = filefd; - ptrdiff_t volatile count_volatile = count; - ptrdiff_t volatile sa_count_volatile = sa_count; - char **volatile new_argv_volatile = new_argv; - - pid = vfork (); child_errno = errno; - - buffer = buffer_volatile; - coding_systems = coding_systems_volatile; - current_dir = current_dir_volatile; - display_p = display_p_volatile; - output_to_buffer = output_to_buffer_volatile; - sa_must_free = sa_must_free_volatile; - fd1 = fd1_volatile; - fd_error = fd_error_volatile; - fd_output = fd_output_volatile; - filefd = filefd_volatile; - count = count_volatile; - sa_count = sa_count_volatile; - new_argv = new_argv_volatile; - - fd0 = synch_process_fd; + unbind_to (count, Qnil); + synchronize_system_messages_locale (); + return + code_convert_string_norecord (build_string (strerror (child_errno)), + Vlocale_coding_system, 0); } + status = pid; - if (pid == 0) + for (i = 0; i < CALLPROC_FDS; i++) + if (0 <= callproc_fd[i]) { - unblock_child_signal (); - - if (fd0 >= 0) - emacs_close (fd0); - - setsid (); - - /* Emacs ignores SIGPIPE, but the child should not. */ - signal (SIGPIPE, SIG_DFL); - - child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir); + emacs_close (callproc_fd[i]); + callproc_fd[i] = -1; } + emacs_close (filefd); + clear_unwind_protect (count - 1); + + if (tempfile) + { + /* Since CRLF is converted to LF within `decode_coding', we + can always open a file with binary mode. */ + callproc_fd[CALLPROC_PIPEREAD] = emacs_open (tempfile, + O_RDONLY | O_BINARY, 0); + if (callproc_fd[CALLPROC_PIPEREAD] < 0) + { + int open_errno = errno; + report_file_errno ("Cannot re-open temporary file", + build_string (tempfile), open_errno); + } + } + +#endif /* MSDOS */ + + /* Do the unwind-protect now, even though the pid is not known, so + that no storage allocation is done in the critical section. + The actual PID will be filled in during the critical section. */ + record_unwind_protect (call_process_cleanup, Fcurrent_buffer ()); + +#ifndef MSDOS + + block_input (); + block_child_signal (); + +#ifdef WINDOWSNT + pid = child_setup (filefd, fd_output, fd_error, new_argv, 0, current_dir); +#else /* not WINDOWSNT */ + + /* vfork, and prevent local vars from being clobbered by the vfork. */ + { + Lisp_Object volatile buffer_volatile = buffer; + Lisp_Object volatile coding_systems_volatile = coding_systems; + Lisp_Object volatile current_dir_volatile = current_dir; + bool volatile display_p_volatile = display_p; + bool volatile sa_must_free_volatile = sa_must_free; + int volatile fd_error_volatile = fd_error; + int volatile filefd_volatile = filefd; + ptrdiff_t volatile count_volatile = count; + ptrdiff_t volatile sa_count_volatile = sa_count; + char **volatile new_argv_volatile = new_argv; + int volatile callproc_fd_volatile[CALLPROC_FDS]; + for (i = 0; i < CALLPROC_FDS; i++) + callproc_fd_volatile[i] = callproc_fd[i]; + + pid = vfork (); + + buffer = buffer_volatile; + coding_systems = coding_systems_volatile; + current_dir = current_dir_volatile; + display_p = display_p_volatile; + sa_must_free = sa_must_free_volatile; + fd_error = fd_error_volatile; + filefd = filefd_volatile; + count = count_volatile; + sa_count = sa_count_volatile; + new_argv = new_argv_volatile; + + for (i = 0; i < CALLPROC_FDS; i++) + callproc_fd[i] = callproc_fd_volatile[i]; + fd_output = callproc_fd[CALLPROC_STDOUT]; + } + + if (pid == 0) + { + unblock_child_signal (); + + setsid (); + + /* Emacs ignores SIGPIPE, but the child should not. */ + signal (SIGPIPE, SIG_DFL); + + child_setup (filefd, fd_output, fd_error, new_argv, 0, current_dir); + } #endif /* not WINDOWSNT */ - child_errno = errno; - - if (pid > 0) - { - if (INTEGERP (buffer)) - record_deleted_pid (pid); - else - synch_process_pid = pid; - } - - unblock_child_signal (); - unblock_input (); - - /* The MSDOS case did this already. */ - if (fd_error >= 0) - emacs_close (fd_error); + child_errno = errno; + + if (pid > 0) + synch_process_pid = pid; + + unblock_child_signal (); + unblock_input (); + #endif /* not MSDOS */ - /* Close most of our file descriptors, but not fd0 - since we will use that to read input from. */ - emacs_close (filefd); - if (fd_output >= 0) - emacs_close (fd_output); - if (fd1 >= 0 && fd1 != fd_error) - emacs_close (fd1); - } - if (pid < 0) report_file_errno ("Doing vfork", Qnil, child_errno); + /* Close our file descriptors, except for callproc_fd[CALLPROC_PIPEREAD] + since we will use that to read input from. */ + for (i = 0; i < CALLPROC_FDS; i++) + if (i != CALLPROC_PIPEREAD && 0 <= callproc_fd[i]) + { + emacs_close (callproc_fd[i]); + callproc_fd[i] = -1; + } + emacs_close (filefd); + clear_unwind_protect (count - 1); + if (INTEGERP (buffer)) return unbind_to (count, Qnil); if (BUFFERP (buffer)) Fset_buffer (buffer); - if (NILP (buffer)) - { - /* If BUFFER is nil, we must read process output once and then - discard it, so setup coding system but with nil. */ - setup_coding_system (Qnil, &process_coding); - process_coding.dst_multibyte = 0; - } - else + fd0 = callproc_fd[CALLPROC_PIPEREAD]; + + if (0 <= fd0) { Lisp_Object val, *args2; @@ -762,13 +726,13 @@ setup_coding_system (val, &process_coding); process_coding.dst_multibyte = ! NILP (BVAR (current_buffer, enable_multibyte_characters)); + process_coding.src_multibyte = 0; } - process_coding.src_multibyte = 0; immediate_quit = 1; QUIT; - if (output_to_buffer) + if (0 <= fd0) { enum { CALLPROC_BUFFER_SIZE_MIN = 16 * 1024 }; enum { CALLPROC_BUFFER_SIZE_MAX = 4 * CALLPROC_BUFFER_SIZE_MIN }; @@ -779,9 +743,8 @@ EMACS_INT total_read = 0; int carryover = 0; bool display_on_the_fly = display_p; - struct coding_system saved_coding; + struct coding_system saved_coding = process_coding; - saved_coding = process_coding; while (1) { /* Repeatedly read until we've filled as much as possible @@ -812,58 +775,55 @@ /* Now NREAD is the total amount of data in the buffer. */ immediate_quit = 0; - if (!NILP (buffer)) - { - if (NILP (BVAR (current_buffer, enable_multibyte_characters)) - && ! CODING_MAY_REQUIRE_DECODING (&process_coding)) - insert_1_both (buf, nread, nread, 0, 1, 0); - else - { /* We have to decode the input. */ - Lisp_Object curbuf; - ptrdiff_t count1 = SPECPDL_INDEX (); - - XSETBUFFER (curbuf, current_buffer); - /* We cannot allow after-change-functions be run - during decoding, because that might modify the - buffer, while we rely on process_coding.produced to - faithfully reflect inserted text until we - TEMP_SET_PT_BOTH below. */ - specbind (Qinhibit_modification_hooks, Qt); - decode_coding_c_string (&process_coding, - (unsigned char *) buf, nread, curbuf); - unbind_to (count1, Qnil); - if (display_on_the_fly - && CODING_REQUIRE_DETECTION (&saved_coding) - && ! CODING_REQUIRE_DETECTION (&process_coding)) - { - /* We have detected some coding system. But, - there's a possibility that the detection was - done by insufficient data. So, we give up - displaying on the fly. */ - if (process_coding.produced > 0) - del_range_2 (process_coding.dst_pos, - process_coding.dst_pos_byte, - process_coding.dst_pos - + process_coding.produced_char, - process_coding.dst_pos_byte - + process_coding.produced, 0); - display_on_the_fly = 0; - process_coding = saved_coding; - carryover = nread; - /* This is to make the above condition always - fails in the future. */ - saved_coding.common_flags - &= ~CODING_REQUIRE_DETECTION_MASK; - continue; - } - - TEMP_SET_PT_BOTH (PT + process_coding.produced_char, - PT_BYTE + process_coding.produced); - carryover = process_coding.carryover_bytes; - if (carryover > 0) - memcpy (buf, process_coding.carryover, - process_coding.carryover_bytes); + if (NILP (BVAR (current_buffer, enable_multibyte_characters)) + && ! CODING_MAY_REQUIRE_DECODING (&process_coding)) + insert_1_both (buf, nread, nread, 0, 1, 0); + else + { /* We have to decode the input. */ + Lisp_Object curbuf; + ptrdiff_t count1 = SPECPDL_INDEX (); + + XSETBUFFER (curbuf, current_buffer); + /* We cannot allow after-change-functions be run + during decoding, because that might modify the + buffer, while we rely on process_coding.produced to + faithfully reflect inserted text until we + TEMP_SET_PT_BOTH below. */ + specbind (Qinhibit_modification_hooks, Qt); + decode_coding_c_string (&process_coding, + (unsigned char *) buf, nread, curbuf); + unbind_to (count1, Qnil); + if (display_on_the_fly + && CODING_REQUIRE_DETECTION (&saved_coding) + && ! CODING_REQUIRE_DETECTION (&process_coding)) + { + /* We have detected some coding system. But, + there's a possibility that the detection was + done by insufficient data. So, we give up + displaying on the fly. */ + if (process_coding.produced > 0) + del_range_2 (process_coding.dst_pos, + process_coding.dst_pos_byte, + process_coding.dst_pos + + process_coding.produced_char, + process_coding.dst_pos_byte + + process_coding.produced, 0); + display_on_the_fly = 0; + process_coding = saved_coding; + carryover = nread; + /* This is to make the above condition always + fails in the future. */ + saved_coding.common_flags + &= ~CODING_REQUIRE_DETECTION_MASK; + continue; } + + TEMP_SET_PT_BOTH (PT + process_coding.produced_char, + PT_BYTE + process_coding.produced); + carryover = process_coding.carryover_bytes; + if (carryover > 0) + memcpy (buf, process_coding.carryover, + process_coding.carryover_bytes); } if (process_coding.mode & CODING_MODE_LAST_BLOCK) @@ -901,7 +861,7 @@ #ifndef MSDOS /* Wait for it to terminate, unless it already has. */ - wait_for_termination (pid, &status, !output_to_buffer); + wait_for_termination (pid, &status, fd0 < 0); #endif immediate_quit = 0; @@ -931,37 +891,19 @@ return make_number (WEXITSTATUS (status)); } -static void -delete_temp_file (Lisp_Object name) -{ - /* Suppress jka-compr handling, etc. */ - ptrdiff_t count = SPECPDL_INDEX (); - specbind (intern ("file-name-handler-alist"), Qnil); -#ifdef WINDOWSNT - /* If this is called when the subprocess didn't exit yet, the - attempt to delete its input file will fail. In that case, we - schedule the file for deletion when the subprocess exits. This - is the 2nd part of handling this situation; see the call to - record_infile in call-process above, for the first part. */ - if (!internal_delete_file (name)) - { - Lisp_Object encoded_file = ENCODE_FILE (name); - - record_pending_deletion (SSDATA (encoded_file)); - } -#else - internal_delete_file (name); -#endif - unbind_to (count, Qnil); -} - /* Create a temporary file suitable for storing the input data of call-process-region. NARGS and ARGS are the same as for - call-process-region. */ + call-process-region. Store into *FILENAME_STRING_PTR a Lisp string + naming the file, and return a read-write file desciptor + for the file. Unwind-protect the file, so that the file descriptor + will be closed and the file removed when the caller unwinds the + specpdl stack. */ -static Lisp_Object -create_temp_file (ptrdiff_t nargs, Lisp_Object *args) +static int +create_temp_file (ptrdiff_t nargs, Lisp_Object *args, + Lisp_Object *filename_string_ptr) { + int fd; struct gcpro gcpro1; Lisp_Object filename_string; Lisp_Object val, start, end; @@ -988,6 +930,7 @@ { Lisp_Object pattern = Fexpand_file_name (Vtemp_file_name_pattern, tmpdir); char *tempfile; + ptrdiff_t count; #ifdef WINDOWSNT /* Cannot use the result of Fexpand_file_name, because it @@ -1008,15 +951,14 @@ GCPRO1 (filename_string); tempfile = SSDATA (filename_string); - { - int fd = mkostemp (tempfile, O_CLOEXEC); - if (fd < 0) - report_file_error ("Failed to open temporary file using pattern", - pattern); - emacs_close (fd); - } - - record_unwind_protect (delete_temp_file, filename_string); + count = SPECPDL_INDEX (); + record_unwind_protect_nothing (); + fd = mkostemp (tempfile, O_CLOEXEC); + if (fd < 0) + report_file_error ("Failed to open temporary file using pattern", + pattern); + set_unwind_protect (count, delete_temp_file, filename_string); + record_unwind_protect_int (close_file_unwind, fd); } start = args[0]; @@ -1047,15 +989,19 @@ /* POSIX lets mk[s]temp use "."; don't invoke jka-compr if we happen to get a ".Z" suffix. */ specbind (intern ("file-name-handler-alist"), Qnil); - Fwrite_region (start, end, filename_string, Qnil, Qlambda, Qnil, Qnil); + write_region (start, end, filename_string, Qnil, Qlambda, Qnil, Qnil, fd); unbind_to (count1, Qnil); } + if (lseek (fd, 0, SEEK_SET) < 0) + report_file_error ("Setting file position", filename_string); + /* Note that Fcall_process takes care of binding coding-system-for-read. */ - RETURN_UNGCPRO (filename_string); + *filename_string_ptr = filename_string; + RETURN_UNGCPRO (fd); } DEFUN ("call-process-region", Fcall_process_region, Scall_process_region, @@ -1085,12 +1031,13 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &rest ARGS) */) (ptrdiff_t nargs, Lisp_Object *args) { - struct gcpro gcpro1; - Lisp_Object infile; + struct gcpro gcpro1, gcpro2; + Lisp_Object infile, val; ptrdiff_t count = SPECPDL_INDEX (); Lisp_Object start = args[0]; Lisp_Object end = args[1]; bool empty_input; + int fd; if (STRINGP (start)) empty_input = SCHARS (start) == 0; @@ -1104,8 +1051,19 @@ empty_input = XINT (start) == XINT (end); } - infile = empty_input ? Qnil : create_temp_file (nargs, args); - GCPRO1 (infile); + if (empty_input) + { + infile = Qnil; + fd = emacs_open (NULL_DEVICE, O_RDONLY, 0); + if (fd < 0) + report_file_error ("Opening null device", Qnil); + record_unwind_protect_int (close_file_unwind, fd); + } + else + fd = create_temp_file (nargs, args, &infile); + + val = infile; + GCPRO2 (infile, val); if (nargs > 3 && !NILP (args[3])) Fdelete_region (start, end); @@ -1122,7 +1080,17 @@ } args[1] = infile; - RETURN_UNGCPRO (unbind_to (count, Fcall_process (nargs, args))); + val = call_process (nargs, args, fd); + + if (!empty_input && 4 < nargs + && (INTEGERP (CONSP (args[4]) ? XCAR (args[4]) : args[4]))) + { + record_deleted_pid (synch_process_pid, infile); + synch_process_pid = 0; + clear_unwind_protect (count); + } + + RETURN_UNGCPRO (unbind_to (count, val)); } #ifndef WINDOWSNT @@ -1683,6 +1651,11 @@ #endif staticpro (&Vtemp_file_name_pattern); +#ifdef MSDOS + synch_process_tempfile = make_number (0); + staticpro (&synch_process_tempfile); +#endif + DEFVAR_LISP ("shell-file-name", Vshell_file_name, doc: /* File name to load inferior shells from. Initialized from the SHELL environment variable, or to a system-dependent === modified file 'src/eval.c' --- src/eval.c 2013-08-02 21:16:33 +0000 +++ src/eval.c 2013-08-06 11:47:10 +0000 @@ -3302,6 +3302,16 @@ previous value without invoking it. */ void +set_unwind_protect (ptrdiff_t count, void (*func) (Lisp_Object), + Lisp_Object arg) +{ + union specbinding *p = specpdl + count; + p->unwind.kind = SPECPDL_UNWIND; + p->unwind.func = func; + p->unwind.arg = arg; +} + +void set_unwind_protect_ptr (ptrdiff_t count, void (*func) (void *), void *arg) { union specbinding *p = specpdl + count; === modified file 'src/fileio.c' --- src/fileio.c 2013-08-05 04:14:43 +0000 +++ src/fileio.c 2013-08-06 11:47:10 +0000 @@ -4746,25 +4746,39 @@ This calls `write-region-annotate-functions' at the start, and `write-region-post-annotation-function' at the end. */) - (Lisp_Object start, Lisp_Object end, Lisp_Object filename, Lisp_Object append, Lisp_Object visit, Lisp_Object lockname, Lisp_Object mustbenew) -{ - int desc; + (Lisp_Object start, Lisp_Object end, Lisp_Object filename, Lisp_Object append, + Lisp_Object visit, Lisp_Object lockname, Lisp_Object mustbenew) +{ + return write_region (start, end, filename, append, visit, lockname, mustbenew, + -1); +} + +/* Like Fwrite_region, except that if DESC is nonnegative, it is a file + descriptor for FILENAME, so do not open or close FILENAME. */ + +Lisp_Object +write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename, + Lisp_Object append, Lisp_Object visit, Lisp_Object lockname, + Lisp_Object mustbenew, int desc) +{ int open_flags; int mode; off_t offset IF_LINT (= 0); + bool open_and_close_file = desc < 0; bool ok; int save_errno = 0; const char *fn; struct stat st; EMACS_TIME modtime; ptrdiff_t count = SPECPDL_INDEX (); - ptrdiff_t count1; + ptrdiff_t count1 IF_LINT (= 0); Lisp_Object handler; Lisp_Object visit_file; Lisp_Object annotations; Lisp_Object encoded_filename; bool visiting = (EQ (visit, Qt) || STRINGP (visit)); bool quietly = !NILP (visit); + bool file_locked = 0; struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5; struct buffer *given_buffer; struct coding_system coding; @@ -4832,7 +4846,6 @@ record_unwind_protect (build_annotations_unwind, Vwrite_region_annotation_buffers); Vwrite_region_annotation_buffers = list1 (Fcurrent_buffer ()); - count1 = SPECPDL_INDEX (); given_buffer = current_buffer; @@ -4871,8 +4884,11 @@ coding.mode |= CODING_MODE_SELECTIVE_DISPLAY; #ifdef CLASH_DETECTION - if (!auto_saving) - lock_file (lockname); + if (open_and_close_file && !auto_saving) + { + lock_file (lockname); + file_locked = 1; + } #endif /* CLASH_DETECTION */ encoded_filename = ENCODE_FILE (filename); @@ -4889,20 +4905,24 @@ mode = auto_saving ? auto_save_mode_bits : 0666; #endif - desc = emacs_open (fn, open_flags, mode); - - if (desc < 0) + if (open_and_close_file) { - int open_errno = errno; + desc = emacs_open (fn, open_flags, mode); + if (desc < 0) + { + int open_errno = errno; #ifdef CLASH_DETECTION - if (!auto_saving) unlock_file (lockname); + if (file_locked) + unlock_file (lockname); #endif /* CLASH_DETECTION */ - UNGCPRO; - report_file_errno ("Opening output file", filename, open_errno); + UNGCPRO; + report_file_errno ("Opening output file", filename, open_errno); + } + + count1 = SPECPDL_INDEX (); + record_unwind_protect_int (close_file_unwind, desc); } - record_unwind_protect_int (close_file_unwind, desc); - if (NUMBERP (append)) { off_t ret = lseek (desc, offset, SEEK_SET); @@ -4910,7 +4930,8 @@ { int lseek_errno = errno; #ifdef CLASH_DETECTION - if (!auto_saving) unlock_file (lockname); + if (file_locked) + unlock_file (lockname); #endif /* CLASH_DETECTION */ UNGCPRO; report_file_errno ("Lseek error", filename, lseek_errno); @@ -4945,9 +4966,9 @@ immediate_quit = 0; - /* fsync is not crucial for auto-save files, since they might lose - some work anyway. */ - if (!auto_saving && !write_region_inhibit_fsync) + /* fsync is not crucial for temporary files. Nor for auto-save + files, since they might lose some work anyway. */ + if (open_and_close_file && !auto_saving && !write_region_inhibit_fsync) { /* Transfer data and metadata to disk, retrying if interrupted. fsync can report a write failure here, e.g., due to disk full @@ -4971,12 +4992,15 @@ ok = 0, save_errno = errno; } - /* NFS can report a write failure now. */ - if (emacs_close (desc) < 0) - ok = 0, save_errno = errno; + if (open_and_close_file) + { + /* NFS can report a write failure now. */ + if (emacs_close (desc) < 0) + ok = 0, save_errno = errno; - /* Discard the unwind protect for close_file_unwind. */ - specpdl_ptr = specpdl + count1; + /* Discard the unwind protect for close_file_unwind. */ + specpdl_ptr = specpdl + count1; + } /* Some file systems have a bug where st_mtime is not updated properly after a write. For example, CIFS might not see the @@ -5052,7 +5076,7 @@ unbind_to (count, Qnil); #ifdef CLASH_DETECTION - if (!auto_saving) + if (file_locked) unlock_file (lockname); #endif /* CLASH_DETECTION */ === modified file 'src/lisp.h' --- src/lisp.h 2013-08-06 05:30:18 +0000 +++ src/lisp.h 2013-08-06 11:47:10 +0000 @@ -3746,11 +3746,12 @@ Lisp_Object, Lisp_Object (*) (Lisp_Object, ptrdiff_t, Lisp_Object *)); extern void specbind (Lisp_Object, Lisp_Object); extern void record_unwind_protect (void (*) (Lisp_Object), Lisp_Object); +extern void record_unwind_protect_ptr (void (*) (void *), void *); extern void record_unwind_protect_int (void (*) (int), int); -extern void record_unwind_protect_ptr (void (*) (void *), void *); extern void record_unwind_protect_void (void (*) (void)); extern void record_unwind_protect_nothing (void); extern void clear_unwind_protect (ptrdiff_t); +extern void set_unwind_protect (ptrdiff_t, void (*) (Lisp_Object), Lisp_Object); extern void set_unwind_protect_ptr (ptrdiff_t, void (*) (void *), void *); extern Lisp_Object unbind_to (ptrdiff_t, Lisp_Object); extern _Noreturn void error (const char *, ...) ATTRIBUTE_FORMAT_PRINTF (1, 2); @@ -3830,6 +3831,9 @@ extern Lisp_Object Qinsert_file_contents; extern Lisp_Object Qfile_name_history; extern Lisp_Object expand_and_dir_to_file (Lisp_Object, Lisp_Object); +extern Lisp_Object write_region (Lisp_Object, Lisp_Object, Lisp_Object, + Lisp_Object, Lisp_Object, Lisp_Object, + Lisp_Object, int); EXFUN (Fread_file_name, 6); /* Not a normal DEFUN. */ extern void close_file_unwind (int); extern void fclose_unwind (void *); === modified file 'src/process.c' --- src/process.c 2013-08-06 14:17:25 +0000 +++ src/process.c 2013-08-06 16:32:10 +0000 @@ -92,6 +92,7 @@ #include <c-ctype.h> #include <sig2str.h> +#include <verify.h> #endif /* subprocesses */ @@ -722,6 +723,8 @@ non-Lisp data, so do it only for slots which should not be zero. */ p->infd = -1; p->outfd = -1; + for (i = 0; i < PROCESS_OPEN_FDS; i++) + p->open_fd[i] = -1; #ifdef HAVE_GNUTLS p->gnutls_initstage = GNUTLS_STAGE_EMPTY; @@ -818,13 +821,17 @@ treated by the SIGCHLD handler and waitpid has been invoked on them; otherwise they might fill up the kernel's process table. - Some processes created by call-process are also put onto this list. */ + Some processes created by call-process are also put onto this list. + + Members of this list are (process-ID . filename) pairs. The + process-ID is a number; the filename, if a string, is a file that + needs to be removed after the process exits. */ static Lisp_Object deleted_pid_list; void -record_deleted_pid (pid_t pid) +record_deleted_pid (pid_t pid, Lisp_Object filename) { - deleted_pid_list = Fcons (make_fixnum_or_float (pid), + deleted_pid_list = Fcons (Fcons (make_fixnum_or_float (pid), filename), /* GC treated elements set to nil. */ Fdelq (Qnil, deleted_pid_list)); @@ -852,7 +859,7 @@ else { if (p->alive) - record_kill_process (p); + record_kill_process (p, Qnil); if (p->infd >= 0) { @@ -1605,17 +1612,45 @@ remove_process (proc); } +/* If *FD_ADDR is nonnegative, close it, and mark it as closed. */ + +static void +close_process_fd (int *fd_addr) +{ + int fd = *fd_addr; + if (0 <= fd) + { + *fd_addr = -1; + emacs_close (fd); + } +} + +/* Indexes of file descriptors in open_fds. */ +enum + { + /* The pipe from Emacs to its subprocess. */ + SUBPROCESS_STDIN, + WRITE_TO_SUBPROCESS, + + /* The main pipe from the subprocess to Emacs. */ + READ_FROM_SUBPROCESS, + SUBPROCESS_STDOUT, + + /* The pipe from the subprocess to Emacs that is closed when the + subprocess execs. */ + READ_FROM_EXEC_MONITOR, + EXEC_MONITOR_OUTPUT + }; + +verify (PROCESS_OPEN_FDS == EXEC_MONITOR_OUTPUT + 1); static void create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) { + struct Lisp_Process *p = XPROCESS (process); int inchannel, outchannel; pid_t pid; int vfork_errno; - int sv[2]; -#ifndef WINDOWSNT - int wait_child_setup[2]; -#endif int forkin, forkout; bool pty_flag = 0; char pty_name[PTY_NAME_SIZE]; @@ -1629,6 +1664,7 @@ if (inchannel >= 0) { + p->open_fd[READ_FROM_SUBPROCESS] = inchannel; #if ! defined (USG) || defined (USG_SUBTTY_WORKS) /* On most USG systems it does not work to open the pty's tty here, then close it and reopen it in the child. */ @@ -1637,6 +1673,7 @@ forkout = forkin = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0); if (forkin < 0) report_file_error ("Opening pty", Qnil); + p->open_fd[SUBPROCESS_STDIN] = forkin; #else forkin = forkout = -1; #endif /* not USG, or USG_SUBTTY_WORKS */ @@ -1645,23 +1682,17 @@ } else { - if (emacs_pipe (sv) != 0) + if (emacs_pipe (p->open_fd + SUBPROCESS_STDIN) != 0 + || emacs_pipe (p->open_fd + READ_FROM_SUBPROCESS) != 0) report_file_error ("Creating pipe", Qnil); - inchannel = sv[0]; - forkout = sv[1]; - if (emacs_pipe (sv) != 0) - { - int pipe_errno = errno; - emacs_close (inchannel); - emacs_close (forkout); - report_file_errno ("Creating pipe", Qnil, pipe_errno); - } - outchannel = sv[1]; - forkin = sv[0]; + forkin = p->open_fd[SUBPROCESS_STDIN]; + outchannel = p->open_fd[WRITE_TO_SUBPROCESS]; + inchannel = p->open_fd[READ_FROM_SUBPROCESS]; + forkout = p->open_fd[SUBPROCESS_STDOUT]; } #ifndef WINDOWSNT - if (emacs_pipe (wait_child_setup) != 0) + if (emacs_pipe (p->open_fd + READ_FROM_EXEC_MONITOR) != 0) report_file_error ("Creating pipe", Qnil); #endif @@ -1670,16 +1701,16 @@ /* Record this as an active process, with its channels. */ chan_process[inchannel] = process; - XPROCESS (process)->infd = inchannel; - XPROCESS (process)->outfd = outchannel; + p->infd = inchannel; + p->outfd = outchannel; /* Previously we recorded the tty descriptor used in the subprocess. It was only used for getting the foreground tty process, so now we just reopen the device (see emacs_get_tty_pgrp) as this is more portable (see USG_SUBTTY_WORKS above). */ - XPROCESS (process)->pty_flag = pty_flag; - pset_status (XPROCESS (process), Qrun); + p->pty_flag = pty_flag; + pset_status (p, Qrun); FD_SET (inchannel, &input_wait_mask); FD_SET (inchannel, &non_keyboard_wait_mask); @@ -1699,25 +1730,21 @@ { Lisp_Object volatile encoded_current_dir_volatile = encoded_current_dir; Lisp_Object volatile lisp_pty_name_volatile = lisp_pty_name; - Lisp_Object volatile process_volatile = process; char **volatile new_argv_volatile = new_argv; int volatile forkin_volatile = forkin; int volatile forkout_volatile = forkout; - int volatile wait_child_setup_0_volatile = wait_child_setup[0]; - int volatile wait_child_setup_1_volatile = wait_child_setup[1]; + struct Lisp_Process *p_volatile = p; pid = vfork (); encoded_current_dir = encoded_current_dir_volatile; lisp_pty_name = lisp_pty_name_volatile; - process = process_volatile; new_argv = new_argv_volatile; forkin = forkin_volatile; forkout = forkout_volatile; - wait_child_setup[0] = wait_child_setup_0_volatile; - wait_child_setup[1] = wait_child_setup_1_volatile; + p = p_volatile; - pty_flag = XPROCESS (process)->pty_flag; + pty_flag = p->pty_flag; } if (pid == 0) @@ -1833,42 +1860,42 @@ /* Back in the parent process. */ vfork_errno = errno; - XPROCESS (process)->pid = pid; + p->pid = pid; if (pid >= 0) - XPROCESS (process)->alive = 1; + p->alive = 1; /* Stop blocking in the parent. */ unblock_child_signal (); unblock_input (); - if (forkin >= 0) - emacs_close (forkin); - if (forkin != forkout && forkout >= 0) - emacs_close (forkout); - if (pid < 0) report_file_errno ("Doing vfork", Qnil, vfork_errno); else { /* vfork succeeded. */ + /* Close the pipe ends that the child uses, or the child's pty. */ + close_process_fd (&p->open_fd[SUBPROCESS_STDIN]); + close_process_fd (&p->open_fd[SUBPROCESS_STDOUT]); + #ifdef WINDOWSNT register_child (pid, inchannel); #endif /* WINDOWSNT */ - pset_tty_name (XPROCESS (process), lisp_pty_name); + pset_tty_name (p, lisp_pty_name); #ifndef WINDOWSNT /* Wait for child_setup to complete in case that vfork is - actually defined as fork. The descriptor wait_child_setup[1] + actually defined as fork. The descriptor + XPROCESS (proc)->open_fd[EXEC_MOINTOR_OUTPUT] of a pipe is closed at the child side either by close-on-exec on successful execve or the _exit call in child_setup. */ { char dummy; - emacs_close (wait_child_setup[1]); - emacs_read (wait_child_setup[0], &dummy, 1); - emacs_close (wait_child_setup[0]); + close_process_fd (&p->open_fd[EXEC_MONITOR_OUTPUT]); + emacs_read (p->open_fd[READ_FROM_EXEC_MONITOR], &dummy, 1); + close_process_fd (&p->open_fd[READ_FROM_EXEC_MONITOR]); } #endif } @@ -1877,16 +1904,13 @@ static void create_pty (Lisp_Object process) { + struct Lisp_Process *p = XPROCESS (process); char pty_name[PTY_NAME_SIZE]; - int inchannel, outchannel; - - inchannel = outchannel = -1; - - if (!NILP (Vprocess_connection_type)) - outchannel = inchannel = allocate_pty (pty_name); - - if (inchannel >= 0) + int pty_fd = NILP (Vprocess_connection_type) ? -1 : allocate_pty (pty_name); + + if (pty_fd >= 0) { + p->open_fd[SUBPROCESS_STDIN] = pty_fd; #if ! defined (USG) || defined (USG_SUBTTY_WORKS) /* On most USG systems it does not work to open the pty's tty here, then close it and reopen it in the child. */ @@ -1895,6 +1919,7 @@ int forkout = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0); if (forkout < 0) report_file_error ("Opening pty", Qnil); + p->open_fd[WRITE_TO_SUBPROCESS] = forkout; #if defined (DONT_REOPEN_PTY) /* In the case that vfork is defined as fork, the parent process (Emacs) may send some data before the child process completes @@ -1903,33 +1928,32 @@ #endif /* DONT_REOPEN_PTY */ #endif /* not USG, or USG_SUBTTY_WORKS */ - fcntl (inchannel, F_SETFL, O_NONBLOCK); - fcntl (outchannel, F_SETFL, O_NONBLOCK); + fcntl (pty_fd, F_SETFL, O_NONBLOCK); /* Record this as an active process, with its channels. As a result, child_setup will close Emacs's side of the pipes. */ - chan_process[inchannel] = process; - XPROCESS (process)->infd = inchannel; - XPROCESS (process)->outfd = outchannel; + chan_process[pty_fd] = process; + p->infd = pty_fd; + p->outfd = pty_fd; /* Previously we recorded the tty descriptor used in the subprocess. It was only used for getting the foreground tty process, so now we just reopen the device (see emacs_get_tty_pgrp) as this is more portable (see USG_SUBTTY_WORKS above). */ - XPROCESS (process)->pty_flag = 1; - pset_status (XPROCESS (process), Qrun); + p->pty_flag = 1; + pset_status (p, Qrun); setup_process_coding_systems (process); - FD_SET (inchannel, &input_wait_mask); - FD_SET (inchannel, &non_keyboard_wait_mask); - if (inchannel > max_process_desc) - max_process_desc = inchannel; + FD_SET (pty_fd, &input_wait_mask); + FD_SET (pty_fd, &non_keyboard_wait_mask); + if (pty_fd > max_process_desc) + max_process_desc = pty_fd; - pset_tty_name (XPROCESS (process), build_string (pty_name)); + pset_tty_name (p, build_string (pty_name)); } - XPROCESS (process)->pid = -2; + p->pid = -2; } @@ -2535,6 +2559,7 @@ p = XPROCESS (proc); fd = serial_open (port); + p->open_fd[SUBPROCESS_STDIN] = fd; p->infd = fd; p->outfd = fd; if (fd > max_process_desc) @@ -3297,12 +3322,6 @@ } #endif - /* Discard the unwind protect for closing S, if any. */ - specpdl_ptr = specpdl + count1; - - /* Unwind bind_polling_period and request_sigio. */ - unbind_to (count, Qnil); - if (s < 0) { /* If non-blocking got this far - and failed - assume non-blocking is @@ -3344,8 +3363,17 @@ if ((tem = Fplist_get (contact, QCstop), !NILP (tem))) pset_command (p, Qt); p->pid = 0; + + p->open_fd[SUBPROCESS_STDIN] = inch; p->infd = inch; p->outfd = outch; + + /* Discard the unwind protect for closing S, if any. */ + specpdl_ptr = specpdl + count1; + + /* Unwind bind_polling_period and request_sigio. */ + unbind_to (count, Qnil); + if (is_server && socktype != SOCK_DGRAM) pset_status (p, Qlisten); @@ -3784,17 +3812,15 @@ static void deactivate_process (Lisp_Object proc) { - register int inchannel, outchannel; - register struct Lisp_Process *p = XPROCESS (proc); + int inchannel; + struct Lisp_Process *p = XPROCESS (proc); + int i; #ifdef HAVE_GNUTLS /* Delete GnuTLS structures in PROC, if any. */ emacs_gnutls_deinit (proc); #endif /* HAVE_GNUTLS */ - inchannel = p->infd; - outchannel = p->outfd; - #ifdef ADAPTIVE_READ_BUFFERING if (p->read_output_delay > 0) { @@ -3805,16 +3831,17 @@ } #endif + inchannel = p->infd; + + /* Beware SIGCHLD hereabouts. */ + if (inchannel >= 0) + flush_pending_output (inchannel); + + for (i = 0; i < PROCESS_OPEN_FDS; i++) + close_process_fd (&p->open_fd[i]); + if (inchannel >= 0) { - /* Beware SIGCHLD hereabouts. */ - flush_pending_output (inchannel); - emacs_close (inchannel); - if (outchannel >= 0 && outchannel != inchannel) - emacs_close (outchannel); - - p->infd = -1; - p->outfd = -1; #ifdef DATAGRAM_SOCKETS if (DATAGRAM_CHAN_P (inchannel)) { @@ -4095,6 +4122,7 @@ /* Discard the unwind protect for closing S. */ specpdl_ptr = specpdl + count; + p->open_fd[SUBPROCESS_STDIN] = s; p->infd = s; p->outfd = s; pset_status (p, Qrun); @@ -6014,7 +6042,8 @@ } else { - int old_outfd, new_outfd; + int old_outfd = XPROCESS (proc)->outfd; + int new_outfd; #ifdef HAVE_SHUTDOWN /* If this is a network connection, or socketpair is used @@ -6022,18 +6051,15 @@ (In some old system, shutdown to socketpair doesn't work. Then we just can't win.) */ if (EQ (XPROCESS (proc)->type, Qnetwork) - || XPROCESS (proc)->outfd == XPROCESS (proc)->infd) - shutdown (XPROCESS (proc)->outfd, 1); - /* In case of socketpair, outfd == infd, so don't close it. */ - if (XPROCESS (proc)->outfd != XPROCESS (proc)->infd) - emacs_close (XPROCESS (proc)->outfd); -#else /* not HAVE_SHUTDOWN */ - emacs_close (XPROCESS (proc)->outfd); -#endif /* not HAVE_SHUTDOWN */ + || XPROCESS (proc)->infd == old_outfd) + shutdown (old_outfd, 1); +#endif + close_process_fd (&XPROCESS (proc)->open_fd[WRITE_TO_SUBPROCESS]); new_outfd = emacs_open (NULL_DEVICE, O_WRONLY, 0); if (new_outfd < 0) - emacs_abort (); - old_outfd = XPROCESS (proc)->outfd; + report_file_error ("Opening null device", Qnil); + XPROCESS (proc)->open_fd[WRITE_TO_SUBPROCESS] = new_outfd; + XPROCESS (proc)->outfd = new_outfd; if (!proc_encode_coding_system[new_outfd]) proc_encode_coding_system[new_outfd] @@ -6042,8 +6068,6 @@ = *proc_encode_coding_system[old_outfd]; memset (proc_encode_coding_system[old_outfd], 0, sizeof (struct coding_system)); - - XPROCESS (proc)->outfd = new_outfd; } return process; } @@ -6120,7 +6144,8 @@ bool all_pids_are_fixnums = (MOST_NEGATIVE_FIXNUM <= TYPE_MINIMUM (pid_t) && TYPE_MAXIMUM (pid_t) <= MOST_POSITIVE_FIXNUM); - Lisp_Object xpid = XCAR (tail); + Lisp_Object head = XCAR (tail); + Lisp_Object xpid = XCAR (head); if (all_pids_are_fixnums ? INTEGERP (xpid) : NUMBERP (xpid)) { pid_t deleted_pid; @@ -6129,7 +6154,11 @@ else deleted_pid = XFLOAT_DATA (xpid); if (child_status_changed (deleted_pid, 0, 0)) - XSETCAR (tail, Qnil); + { + if (STRINGP (XCDR (head))) + unlink (SSDATA (XCDR (head))); + XSETCAR (tail, Qnil); + } } } === modified file 'src/process.h' --- src/process.h 2013-07-09 07:04:48 +0000 +++ src/process.h 2013-08-06 11:47:10 +0000 @@ -31,6 +31,11 @@ # define PROCESS_INLINE INLINE #endif +/* Bound on number of file descriptors opened on behalf of a process, + that need to be closed. */ + +enum { PROCESS_OPEN_FDS = 6 }; + /* This structure records information about a subprocess or network connection. */ @@ -115,6 +120,9 @@ int infd; /* Descriptor by which we write to this process */ int outfd; + /* Descriptors that were created for this process and that need + closing. Unused entries are negative. */ + int open_fd[PROCESS_OPEN_FDS]; /* Event-count of last event in which this process changed status. */ EMACS_INT tick; /* Event-count of last such event reported. */ @@ -210,13 +218,16 @@ extern void block_child_signal (void); extern void unblock_child_signal (void); -extern void record_kill_process (struct Lisp_Process *); +extern void record_kill_process (struct Lisp_Process *, Lisp_Object); -/* Defined in process.c. */ +/* Defined in sysdep.c. */ extern Lisp_Object list_system_processes (void); extern Lisp_Object system_process_attributes (Lisp_Object); +/* Defined in process.c. */ + +extern void record_deleted_pid (pid_t, Lisp_Object); extern void hold_keyboard_input (void); extern void unhold_keyboard_input (void); extern bool kbd_on_hold_p (void); === modified file 'src/syswait.h' --- src/syswait.h 2013-01-02 16:13:04 +0000 +++ src/syswait.h 2013-08-06 11:47:10 +0000 @@ -52,9 +52,6 @@ #define WTERMSIG(status) ((status) & 0x7f) #endif -/* Defined in process.c. */ -extern void record_deleted_pid (pid_t); - /* Defined in sysdep.c. */ extern void wait_for_termination (pid_t, int *, bool); extern pid_t child_status_changed (pid_t, int *, int);
[Message part 3 (message/rfc822, inline)]
From: Paul Eggert <paul.eggert <at> verizon.net> To: 15035-done <at> debbugs.gnu.org Subject: Re: Fix some fd issues when running subprocesses Date: Mon, 12 Aug 2013 00:14:03 -0700Installed as trunk bzr 113813, and marking this as done.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.