Re: 2.1.65 snapshot soon?

[John W. Eaton]

On 14-Feb-2005, John W. Eaton <address@hidden> wrote:

| On 14-Feb-2005, Dmitri A. Sergatskov <address@hidden> wrote:
| 
| | The problem is still there.
| | 
| | Moreover,  __kluge_procbuf_delay__=10, does not seems to help anymore.
| | 
| | I just did 'make all' after applying patch and main.o in serc/ is
| | rebuilt. I can do a clean rebuild, but it would take few hours...
| 
| What compiler and version are you using?
| 
| Can you do some debugging and find out whether errno is ever set to
| EAGAIN after a call to read from the stream?
| 
| In the ls function, what is the stream state after the read that
| doesn't return anything?

After much messing around offline, I think we have finally discovered
the real cause and solution to this long-standing problem.  The short
summary is that it was possible for a signal to be delivered while
Octave was reading data, and the read system call was not being
restarted.  Since we are using blocking I/O, we should not need to
check errno for EAGAIN or sleep even a tiny bit to try to ensure that
the child process starts before we try to read from the pipe (the read
operation should block until data is available).  A fix is below.
I've also checked in another small change that makes

 x = ls

not print to octave_stdout.

Thanks to Dmitri and Todd Neal for help in uncovering the real cause
of this bug.

jwe



src/ChangeLog:

2005-02-15  John W. Eaton  <address@hidden>

        * sighandlers.cc (octave_set_signal_handler): Request system calls
        restarted if interrupted by signals (except for SIGALRM).
        * dirfns.cc (Fls): Don't bother with sleeping or checking errno.


Index: src/dirfns.cc
===================================================================
RCS file: /usr/local/cvsroot/octave/src/dirfns.cc,v
retrieving revision 1.87
diff -u -r1.87 dirfns.cc
--- src/dirfns.cc       28 Dec 2004 01:59:05 -0000      1.87
+++ src/dirfns.cc       15 Feb 2005 11:52:16 -0000
@@ -182,16 +182,8 @@
 
   unwind_protect::add (cleanup_iprocstream, cmd);
 
-  // XXX FIXME XXX -- Perhaps we should read more than one character
-  // at a time and find a way to avoid the call to octave_usleep as
-  // well?
-
   if (cmd && *cmd)
     {
-      // This is a bit of a kluge...
-
-      octave_usleep (100);
-
       char ch;
 
       OSSTREAM output_buf;
@@ -204,16 +196,7 @@
              output_buf << ch;
            }
          else
-           {
-             if (! cmd->eof () && errno == EAGAIN)
-               {
-                 cmd->clear ();
-
-                 octave_usleep (100);
-               }
-             else
-               break;
-           }
+           break;
        }
 
       output_buf << OSSTREAM_ENDS;
Index: src/sighandlers.cc
===================================================================
RCS file: /usr/local/cvsroot/octave/src/sighandlers.cc,v
retrieving revision 1.83
diff -u -r1.83 sighandlers.cc
--- src/sighandlers.cc  12 Feb 2005 02:29:35 -0000      1.83
+++ src/sighandlers.cc  15 Feb 2005 11:52:17 -0000
@@ -176,11 +176,28 @@
 {
 #if defined (HAVE_POSIX_SIGNALS)
   struct sigaction act, oact;
+
   act.sa_handler = handler;
   act.sa_flags = 0;
+
+  if (sig == SIGALRM)
+    {
+#if defined (SA_INTERRUPT)
+      act.sa_flags |= SA_INTERRUPT;
+#endif
+    }
+  else
+    {
+#if defined (SA_RESTART)
+      act.sa_flags |= SA_RESTART;
+#endif
+    }
+
   sigemptyset (&act.sa_mask);
   sigemptyset (&oact.sa_mask);
+
   sigaction (sig, &act, &oact);
+
   return oact.sa_handler;
 #else
   return signal (sig, handler);