changeset 5588:2dbefd79b3d3

Prevent SIGPIPEs in deactivate_process(). * process.c (deactivate_process): Use Lstream_close_noflush on output pipe instead of Lstream_close. * lstream.c (Lstream_close_noflush): New. Factored out of Lstream_close. (Lstream_close): Use Lstream_close_noflush. * lstream.h (Lstream_close_noflush): Declare it.
author Stephen J. Turnbull <stephen@xemacs.org>
date Sat, 29 Oct 2011 01:10:32 +0900
parents 3fde0e346ad7
children 4218b56833b3
files src/ChangeLog src/lstream.c src/lstream.h src/process.c
diffstat 4 files changed, 67 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/src/ChangeLog	Sat Oct 29 00:35:33 2011 +0900
+++ b/src/ChangeLog	Sat Oct 29 01:10:32 2011 +0900
@@ -1,3 +1,16 @@
+2011-10-29  Stephen J. Turnbull  <stephen@xemacs.org>
+
+	Prevent SIGPIPEs in deactivate_process().
+
+	* process.c (deactivate_process):
+	Use Lstream_close_noflush on output pipe instead of Lstream_close.
+
+	* lstream.c (Lstream_close_noflush):
+	New.  Factored out of Lstream_close.
+	(Lstream_close): Use Lstream_close_noflush.
+
+	* lstream.h (Lstream_close_noflush): Declare it.
+
 2011-10-29  Stephen J. Turnbull  <stephen@xemacs.org>
 
 	Prevent assert at frame.c, l. 6311 by initializing glyph cachels.
--- a/src/lstream.c	Sat Oct 29 00:35:33 2011 +0900
+++ b/src/lstream.c	Sat Oct 29 01:10:32 2011 +0900
@@ -791,6 +791,45 @@
   return Lstream_flush (lstr);
 }
 
+/* Close the stream without flushing buffers.
+   In current practice, this is only useful when a subprocess terminates
+   unexpectedly, and the OS closes its pipes without warning.  In that case,
+   we do not want to flush our output buffers, as there is no longer a pipe
+   to write to.
+   This nomenclature may deserve review if XEmacs starts getting called as
+   a subprocess. */
+
+int
+Lstream_close_noflush (Lstream *lstr)
+{
+  lstr->flags &= ~LSTREAM_FL_IS_OPEN;
+  lstr->byte_count = 0;
+  /* Note that Lstream_flush() reset all the buffer indices.  That way,
+     the next call to Lstream_putc(), Lstream_getc(), or Lstream_ungetc()
+     on a closed stream will call into the function equivalents, which will
+     cause an error. */
+
+  /* We set the pointers to 0 so that we don't lose when this function
+     is called more than once on the same object */
+  if (lstr->out_buffer)
+    {
+      xfree (lstr->out_buffer);
+      lstr->out_buffer = 0;
+    }
+  if (lstr->in_buffer)
+    {
+      xfree (lstr->in_buffer);
+      lstr->in_buffer = 0;
+    }
+  if (lstr->unget_buffer)
+    {
+      xfree (lstr->unget_buffer);
+      lstr->unget_buffer = 0;
+    }
+
+  return 0;
+}
+
 /* Close the stream.  All data will be flushed out.  If the stream is
    already closed, nothing happens.  Note that, even if all data has
    already been flushed out, the act of closing a stream may generate more
@@ -838,30 +877,7 @@
 	  rc = -1;
     }
 
-  lstr->flags &= ~LSTREAM_FL_IS_OPEN;
-  lstr->byte_count = 0;
-  /* Note that Lstream_flush() reset all the buffer indices.  That way,
-     the next call to Lstream_putc(), Lstream_getc(), or Lstream_ungetc()
-     on a closed stream will call into the function equivalents, which will
-     cause an error. */
-
-  /* We set the pointers to 0 so that we don't lose when this function
-     is called more than once on the same object */
-  if (lstr->out_buffer)
-    {
-      xfree (lstr->out_buffer);
-      lstr->out_buffer = 0;
-    }
-  if (lstr->in_buffer)
-    {
-      xfree (lstr->in_buffer);
-      lstr->in_buffer = 0;
-    }
-  if (lstr->unget_buffer)
-    {
-      xfree (lstr->unget_buffer);
-      lstr->unget_buffer = 0;
-    }
+  Lstream_close_noflush (lstr);
 
   return rc;
 }
--- a/src/lstream.h	Sat Oct 29 00:35:33 2011 +0900
+++ b/src/lstream.h	Sat Oct 29 01:10:32 2011 +0900
@@ -306,6 +306,7 @@
 int Lstream_rewind (Lstream *lstr);
 int Lstream_seekable_p (Lstream *lstr);
 int Lstream_close (Lstream *lstr);
+int Lstream_close_noflush (Lstream *lstr);
 
 void Lstream_delete (Lstream *lstr);
 void Lstream_set_character_mode (Lstream *str);
--- a/src/process.c	Sat Oct 29 00:35:33 2011 +0900
+++ b/src/process.c	Sat Oct 29 01:10:32 2011 +0900
@@ -2150,8 +2150,20 @@
   /* Must call this before setting the streams to nil */
   event_stream_unselect_process (p, 1, 1);
 
+  /* We can get here in case of a crash in the external process, and then
+     the Lstream_close on output will cause a SIGPIPE, which we're not ready
+     for here.  It looks to me like all cases where this function is called
+     we know the process has exited (but I'm not 100% sure for the call in
+     execute_internal_event (event-stream.c)), so it should be OK to use
+     Lstream_close_noflush.
+
+     #### The layering here needs a rethink.  We should just be able
+     to call Lstream_close, and let the Lstream's implementation decide
+     if it can flush safely or not.  The immediate problem is that the
+     Lstream needs to know the process's status, but I don't think it has
+     a handle to the process. */
   if (!NILP (DATA_OUTSTREAM (p)))
-    Lstream_close (XLSTREAM (DATA_OUTSTREAM (p)));
+    Lstream_close_noflush (XLSTREAM (DATA_OUTSTREAM (p)));
   if (!NILP (DATA_INSTREAM (p)))
     Lstream_close (XLSTREAM (DATA_INSTREAM (p)));
   if (!NILP (DATA_ERRSTREAM (p)))