[Emacs-diffs] /srv/bzr/emacs/trunk r111244: Fix bug #13079 on MS-Windows with temp files not being deleted.

[Eli Zaretskii]

------------------------------------------------------------
revno: 111244
fixes bug: http://debbugs.gnu.org/13079
committer: Eli Zaretskii <address@hidden>
branch nick: trunk
timestamp: Sat 2012-12-15 15:38:21 +0200
message:
  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.
modified:
  nt/inc/ms-w32.h
  src/ChangeLog
  src/callproc.c
  src/fileio.c
  src/lisp.h
  src/w32.h
  src/w32proc.c

=== modified file 'nt/inc/ms-w32.h'
--- a/nt/inc/ms-w32.h   2012-12-08 18:27:37 +0000
+++ b/nt/inc/ms-w32.h   2012-12-15 13:38:21 +0000
@@ -178,6 +178,10 @@
 #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
 

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-12-14 20:05:03 +0000
+++ b/src/ChangeLog     2012-12-15 13:38:21 +0000
@@ -1,3 +1,34 @@
+2012-12-15  Eli Zaretskii  <address@hidden>
+
+       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  <address@hidden>
 
        * editfns.c [HAVE_PWD_H]: Include grp.h.

=== modified file 'src/callproc.c'
--- a/src/callproc.c    2012-12-15 11:04:14 +0000
+++ b/src/callproc.c    2012-12-15 13:38:21 +0000
@@ -402,10 +402,8 @@
 
   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 @@
 
 #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 @@
   /* 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;
 }

=== modified file 'src/fileio.c'
--- a/src/fileio.c      2012-12-13 19:35:10 +0000
+++ b/src/fileio.c      2012-12-15 13:38:21 +0000
@@ -2203,14 +2203,17 @@
   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,

=== modified file 'src/lisp.h'
--- a/src/lisp.h        2012-12-10 12:08:02 +0000
+++ b/src/lisp.h        2012-12-15 13:38:21 +0000
@@ -3199,7 +3199,7 @@
 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);

=== modified file 'src/w32.h'
--- a/src/w32.h 2012-12-15 11:04:14 +0000
+++ b/src/w32.h 2012-12-15 13:38:21 +0000
@@ -103,6 +103,12 @@
   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 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 *);

=== modified file 'src/w32proc.c'
--- a/src/w32proc.c     2012-12-15 11:41:18 +0000
+++ b/src/w32proc.c     2012-12-15 13:38:21 +0000
@@ -812,6 +812,8 @@
   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 @@
   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 @@
    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 @@
   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 @@
 #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 @@
   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 @@
 
 /* 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;