From b07286179ec721bc44a7e32111318fb325789fe1 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 15 Dec 2012 15:38:21 +0200 Subject: [PATCH] Fix bug #13079 on MS-Windows with temp files not being deleted. src/w32.h (_child_process): New members input_file and pending_deletion. (register_child): First argument is now pid_t. (record_infile, record_pending_deletion): New prototypes. src/w32proc.c (new_child): Initialize input_file and pending_deletion members of the child. (delete_child): Delete the child's temporary input file, if any, that is pending deletion. (register_child): First argument is now pid_t. (record_infile, record_pending_deletion): New functions. (reap_subprocess): Fix a typo in DebPrint string. (sys_spawnve, sys_kill): Use pid_t for PID arguments. src/fileio.c (internal_delete_file): Return an int again: non-zero if delete-file succeeds, zero otherwise. src/lisp.h (internal_delete_file): Adjust prototype. src/callproc.c (Fcall_process): Don't overwrite infile with result of DECODE_FILE. [WINDOWSNT] If BUFFER is an integer, i.e. we are launching an asynchronous subprocess, record the name of the input file name, if any. (delete_temp_file) [WINDOWSNT]: If internal_delete_file fails to delete the file, record it as pending deletion when the subprocess exits. nt/inc/ms-w32.h (sys_unlink): Provide prototype. --- nt/inc/ms-w32.h | 4 +++ src/ChangeLog | 31 +++++++++++++++++++++++ src/callproc.c | 29 +++++++++++++++++++--- src/fileio.c | 11 ++++++--- src/lisp.h | 2 +- src/w32.h | 10 +++++++- src/w32proc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++---- 7 files changed, 138 insertions(+), 15 deletions(-) diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h index 3d68efb804..aab3039106 100644 --- a/nt/inc/ms-w32.h +++ b/nt/inc/ms-w32.h @@ -178,6 +178,10 @@ extern char *getenv (); #define strerror sys_strerror #undef unlink #define unlink sys_unlink +/* This prototype is needed because some files include config.h + _after_ the standard headers, so sys_unlink gets no prototype from + stdio.h or io.h. */ +extern int sys_unlink (const char *); #undef write #define write sys_write diff --git a/src/ChangeLog b/src/ChangeLog index 960e4e52c5..34959cb691 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,34 @@ +2012-12-15 Eli Zaretskii + + Fix bug #13079 on MS-Windows with temp files not being deleted. + * w32.h (_child_process): New members input_file and + pending_deletion. + (register_child): First argument is now pid_t. + (record_infile, record_pending_deletion): New prototypes. + + * w32proc.c (new_child): Initialize input_file and + pending_deletion members of the child. + (delete_child): Delete the child's temporary input file, if any, + that is pending deletion. + (register_child): First argument is now pid_t. + (record_infile, record_pending_deletion): New functions. + (reap_subprocess): Fix a typo in DebPrint string. + (sys_spawnve, sys_kill): Use pid_t for PID arguments. + + * fileio.c (internal_delete_file): Return an int again: non-zero + if delete-file succeeds, zero otherwise. + + * lisp.h (internal_delete_file): Adjust prototype. + + * callproc.c (Fcall_process): Don't overwrite infile with result + of DECODE_FILE. + [WINDOWSNT] If BUFFER is an integer, i.e. we are launching an + asynchronous subprocess, record the name of the input file name, + if any. + (delete_temp_file) [WINDOWSNT]: If internal_delete_file fails to + delete the file, record it as pending deletion when the subprocess + exits. + 2012-12-14 Eli Zaretskii * editfns.c [HAVE_PWD_H]: Include grp.h. diff --git a/src/callproc.c b/src/callproc.c index e064dccd6d..5eba327135 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -402,10 +402,8 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) filefd = emacs_open (SSDATA (infile), O_RDONLY, 0); if (filefd < 0) - { - infile = DECODE_FILE (infile); - report_file_error ("Opening process input file", Fcons (infile, Qnil)); - } + report_file_error ("Opening process input file", + Fcons (DECODE_FILE (infile), Qnil)); if (STRINGP (output_file)) { @@ -612,6 +610,15 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) #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. */ @@ -924,7 +931,21 @@ 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); return Qnil; } diff --git a/src/fileio.c b/src/fileio.c index c7df87bcdd..9f263e3694 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -2203,14 +2203,17 @@ internal_delete_file_1 (Lisp_Object ignore) return Qt; } -/* Delete file FILENAME. +/* Delete file FILENAME, returning 1 if successful and 0 if failed. This ignores `delete-by-moving-to-trash'. */ -void +int internal_delete_file (Lisp_Object filename) { - internal_condition_case_2 (Fdelete_file, filename, Qnil, - Qt, internal_delete_file_1); + Lisp_Object tem; + + tem = internal_condition_case_2 (Fdelete_file, filename, Qnil, + Qt, internal_delete_file_1); + return NILP (tem); } DEFUN ("rename-file", Frename_file, Srename_file, 2, 3, diff --git a/src/lisp.h b/src/lisp.h index c7a70feced..84a97c17bb 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3199,7 +3199,7 @@ EXFUN (Fread_file_name, 6); /* Not a normal DEFUN. */ extern Lisp_Object close_file_unwind (Lisp_Object); extern Lisp_Object restore_point_unwind (Lisp_Object); extern _Noreturn void report_file_error (const char *, Lisp_Object); -extern void internal_delete_file (Lisp_Object); +extern int internal_delete_file (Lisp_Object); extern bool file_directory_p (const char *); extern bool file_accessible_directory_p (const char *); extern void syms_of_fileio (void); diff --git a/src/w32.h b/src/w32.h index 86f9c992fe..a43a5133eb 100644 --- a/src/w32.h +++ b/src/w32.h @@ -103,6 +103,12 @@ typedef struct _child_process OVERLAPPED ovl_read; /* Used for async write operations on serial comm ports. */ OVERLAPPED ovl_write; + /* Input file, if any, for this subprocess. Should only be non-NULL + for async subprocesses. */ + char *input_file; + /* If non-zero, the subprocess input file is temporary and should be + deleted when the subprocess exits. */ + int pending_deletion; } child_process; #define MAXDESC FD_SETSIZE @@ -184,7 +190,9 @@ extern int sys_pipe (int *); extern void set_process_dir (char *); extern int sys_spawnve (int, char *, char **, char **); -extern void register_child (int, int); +extern void register_child (pid_t, int); +extern void record_infile (pid_t, char *); +extern void record_pending_deletion (char *); extern void sys_sleep (int); extern int sys_link (const char *, const char *); diff --git a/src/w32proc.c b/src/w32proc.c index 3bdd999534..43ecf4d68f 100644 --- a/src/w32proc.c +++ b/src/w32proc.c @@ -812,6 +812,8 @@ new_child (void) cp->pid = -1; cp->procinfo.hProcess = NULL; cp->status = STATUS_READ_ERROR; + cp->input_file = NULL; + cp->pending_deletion = 0; /* use manual reset event so that select() will function properly */ cp->char_avail = CreateEvent (NULL, TRUE, FALSE, NULL); @@ -860,6 +862,21 @@ delete_child (child_process *cp) if (!CHILD_ACTIVE (cp)) return; + /* Delete the child's temporary input file, if any, that is pending + deletion. */ + if (cp->input_file) + { + if (cp->pending_deletion) + { + if (unlink (cp->input_file)) + DebPrint (("delete_child.unlink (%s) failed, errno: %d\n", + cp->input_file, errno)); + cp->pending_deletion = 0; + } + xfree (cp->input_file); + cp->input_file = NULL; + } + /* reap thread if necessary */ if (cp->thrd) { @@ -1056,11 +1073,11 @@ create_child (char *exe, char *cmdline, char *env, int is_gui_app, This way the select emulator knows how to match file handles with entries in child_procs. */ void -register_child (int pid, int fd) +register_child (pid_t pid, int fd) { child_process *cp; - cp = find_child_pid (pid); + cp = find_child_pid ((DWORD)pid); if (cp == NULL) { DebPrint (("register_child unable to find pid %lu\n", pid)); @@ -1087,6 +1104,45 @@ register_child (int pid, int fd) fd_info[fd].cp = cp; } +/* Record INFILE as an input file for process PID. */ +void +record_infile (pid_t pid, char *infile) +{ + child_process *cp; + + /* INFILE should never be NULL, since xstrdup would have signaled + memory full condition in that case, see callproc.c where this + function is called. */ + eassert (infile); + + cp = find_child_pid ((DWORD)pid); + if (cp == NULL) + { + DebPrint (("record_infile is unable to find pid %lu\n", pid)); + return; + } + + cp->input_file = infile; +} + +/* Mark the input file INFILE of the corresponding subprocess as + temporary, to be deleted when the subprocess exits. */ +void +record_pending_deletion (char *infile) +{ + child_process *cp; + + eassert (infile); + + for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) + if (CHILD_ACTIVE (cp) + && cp->input_file && xstrcasecmp (cp->input_file, infile) == 0) + { + cp->pending_deletion = 1; + break; + } +} + /* Called from waitpid when a process exits. */ static void reap_subprocess (child_process *cp) @@ -1097,7 +1153,7 @@ reap_subprocess (child_process *cp) #ifdef FULL_DEBUG /* Process should have already died before we are called. */ if (WaitForSingleObject (cp->procinfo.hProcess, 0) != WAIT_OBJECT_0) - DebPrint (("reap_subprocess: child fpr fd %d has not died yet!", cp->fd)); + DebPrint (("reap_subprocess: child for fd %d has not died yet!", cp->fd)); #endif CloseHandle (cp->procinfo.hProcess); cp->procinfo.hProcess = NULL; @@ -1465,7 +1521,7 @@ sys_spawnve (int mode, char *cmdname, char **argv, char **envp) Lisp_Object program, full; char *cmdline, *env, *parg, **targ; int arglen, numenv; - int pid; + pid_t pid; child_process *cp; int is_dos_app, is_cygnus_app, is_gui_app; int do_quoting = 0; @@ -2129,7 +2185,7 @@ find_child_console (HWND hwnd, LPARAM arg) /* Emulate 'kill', but only for other processes. */ int -sys_kill (int pid, int sig) +sys_kill (pid_t pid, int sig) { child_process *cp; HANDLE proc_hand; -- 2.39.2