Subject: FvwmCommand dying with dead pipe

From: Julian Bradfield <[email-masked]>
Date: Mon, 7 Jun 1999 13:54:01 +0200 (MDT)

Hello,

A few months ago you posted to the fvwm list about a problem with
FvwmCommandS dying with "dead pipe", and the CC'ed people contributed
to the thread.

I tried to use FvwmCommandS yesterday, hit this problem, and found the
thread on Dejanews. Dejanews didn't throw up the more recent messages
(including Toshi's quick fix), so I started debugging. You might be
interested in the results.

I'm also copying to fvwm-workers, who might want to note the last
paragraph; even though I'm not involved with fvwm at all---hope
that's OK.

You didn't say what OS you were using, but I'm betting it's Linux.
It turns out (after about six hours trying to work out what could
possibly be wrong with fvwm :-( ) that there is a bug in all Linux
kernels (at least all 2.X kernels). Namely, if you do a read() of
length zero on a non-empty pipe, it returns an error. This is
blatantly wrong, of course, and is a simple bug in the system code.

This is triggered by the FvwmCommandS line that sends a null message
to fvwm2: fvwm2 reads the command size (0), then does a read of length
zero to get the text. Owing to the kernel bug, this returns error, so
fvwm2 shuts down the connection.
The reason it is (as I found) intermittent, typically going away when
you trace, is that a 0-length read on an *empty* pipe does work: so if
fvwm2 reads the text before FvwmCommandS has sent the "cont" flag, it's
OK.
I don't know why it doesn't seem to have been a major problem with the
2.0 kernels; maybe some artefact of scheduling meant it was rarely
triggered.

I enclose a copy of the message I sent to linux-kernel, and I think
the bug should be fixed in the next kernels. However, as people
probably upgrade fvwm2 more easily than kernels, it might be worthwhile
for the fvwm community to incorporate a work-around, by not doing any
zero length reads in the functions that read from pipes.
(Sorry, no patch: I only have an old version of fvwm2.)

Here's the message I sent to linux-kernel:

Subject: [PROBLEM+PATCH] 2.X zero-length read on non-empty pipe returns EAGAIN
From: Julian Bradfield <[email-masked]>
To: [email-masked]
Date: Mon, 7 Jun 1999 12:19:44 +0200 (MDT)

[Note: I can't find a maintainer for this, which means it's probably
Linus, which means there's no email address to send patches to.
Is this the only place I need to send it? ]

This problem was observed with kernel 2.2.9 on an i386, but appears
from the source to be present in all 2.X kernels.

Description: a read() with count=0 on a pipe that is not empty
should return 0; instead it returns -1 with EAGAIN.

Reproduce by the following program:
----- begin pipebug.c -----
#include <stdio.h>

void main() {
  int fds[2];
  int n; char junk[10];char *data = "0123456789";

  pipe(fds);
  if ( fork() ) {
    close(fds[1]);
    sleep(5); /* just to ensure the write has happened in the child! */
    n = read(fds[0],junk,0);
    printf("Got %d chars\n",n);
    if ( n < 0 ) perror("first read");
    n = read(fds[0],junk,4);
    printf("Got %d chars\n",n);
    if ( n < 0 ) perror("second read");
  } else {
    close(fds[0]);
    write(fds[1],data,4);
    sleep(10) ; /* to avoid closing pipe */
  }
}
----- end pipebug.c -----

Analysis: simple bug in pipe_read() in linux/fs/pipe.c.

Fix: *If* the manual page for read() is correct in saying that 
a read of length zero should always return 0 and do nothing else,
then the enclosed patch is probably right. (Somebody who knows, please
check that it should indeed come after the invalid seek test---I don't
know enough about what the fs routines are defined to do.)
If on the other hand there are circumstances in which such a read
should return an error (e.g. if it is felt that the locking should
still be done), then a slightly less trivial fix is required.

Patch follows:

--- linux-2.2.9/fs/pipe.c	Fri Nov 13 19:07:26 1998
+++ linux/fs/pipe.c	Mon Jun  7 09:08:03 1999
@@ -25,7 +25,7 @@
 /* Florian Coosmann (FGC)                                ^ current = 1       */
 /* Additionally, we now use locking technique. This prevents race condition  */
 /* in case of paging and multiple read/write on the same pipe. (FGC)         */
-
+/* Reads with count = 0 should always return 0. Julian Bradfield 1999-06-07. */
 
 static ssize_t pipe_read(struct file * filp, char * buf,
 			 size_t count, loff_t *ppos)
@@ -34,8 +34,11 @@
 	ssize_t chars = 0, size = 0, read = 0;
         char *pipebuf;
 
+
 	if (ppos != &filp->f_pos)
 		return -ESPIPE;
+
+	if ( !count ) return 0;
 
 	if (filp->f_flags & O_NONBLOCK) {
 		if (PIPE_LOCK(*inode))

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 7 Jun 1999 14:07:58 -0600

> 2.0 kernels; maybe some artefact of scheduling meant it was rarely

Without a similar bug in FvwmCommandS it would never have surfaced.
We were doing a zero length write and were expecting it to succeed,
which - acoording to the man page - it doesn't. Now that we don't do
zero writes FvwmCommandS doesn't get a dead pipe. I think it might be
better to see if more such problems surface that to clutter the fvwm
code with workarounds for bugs in the code of someone else. Once its
in the code we will never be able to remove it again, but if
someone complains we can simply tell him to update his kernel.

Bye

Dominik ^_^

-- 
Dominik Vogt, Hewlett-Packard GmbH, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/14-4596, fax: 07031/14-3883, [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 7 Jun 1999 14:23:16 -0600

> someone complains we can simply tell him to update his kernel.

Oops, sorry, my memory has failed me. The man page says that nothing
is written to a file descriptor for a length of zero.

Bye

Dominik ^_^

---

From: Steve Robbins <[email-masked]>
Date: Mon, 7 Jun 1999 09:25:38 -0400 (EDT)

On Mon, 7 Jun 1999, Julian Bradfield wrote:

> kernels (at least all 2.X kernels). 
                 ^^^^^^^^^^^^^^^^^
You may want to check further.

Your program succeeded on my 2.0 kernel:

steve@hilbert{test}./a.out 
Got 0 chars
Got 4 chars
steve@hilbert{test}uname -a
Linux hilbert.nyongwa.montreal.qc.ca 2.0.35 #10 Fri Aug 7 23:43:46 EDT 1998 i486 unknown

--
Steve Robbins                              <[email-masked]>

When love is gone, there's always justice.
And when justice is gone, there's always force.
And when force is gone, there's always Mom.

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 7 Jun 1999 17:03:31 -0600

> > kernels (at least all 2.X kernels). 

It happens only with 2.2.x, 2.0.x is okay.

Bye

Dominik ^_^

---

From: Julian Bradfield <[email-masked]>
Date: Mon, 7 Jun 1999 17:07:51 +0200 (MDT)

>Your program succeeded on my 2.0 kernel:

Right, I should have read the source more carefully (or tried it!).

In 2.0, the check for count=0 was done by the read system call itself,
but in the recent kernels it's done by the file operations. I wonder
why?

So, my apologies for exaggerating the problem (to all),
and thanks to you for catching me out!
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 8 Jun 1999 11:54:24 -0600

On Tue, Jun 08, 1999 at 11:31:59AM +0200, [email-masked] wrote:
> Dominik Vogt writes:
> >Now that we don't do zero writes FvwmCommandS doesn't get a dead
> >pipe.
> 
> Actually, both these statements are wrong, aren't they?
> The 0-length write is nothing to do with the Linux read() bug, and if
> anything, it slightly decreases the chance of the read() bug being
> triggered, since it slightly increases the chance of the read(..,0)
> happening before cont has been written to the pipe.
> 
> (Maybe there are systems on which a zero-length write on a pipe
> actually does return error; but Linux isn't one of them.)
> 
> In particular, with the fvwm-2.2.2 distribution and a Linux 2.2.5 kernel
> (which I believe is what ships with RedHat 6.0, isn't it?),
> FvwmCommandS gets a dead pipe (on my machine, at least!).

The fix we made disappears once in a while. It's bad news that it didn't
make it into 2.2.

> This can still be worked round by the kludge of having FvwmCommandS.c
> send " " instead of ""; but that's not what 2.2.2 does.
> Which is one reason why I think it would be better to avoid the
> zero-length read in fvwm/module.c HandleModuleInput() ; I'd have
> thought that most people writing such code would do that anyway, even
> if they knew that 0-length reads were defined to succeed---it's
> harmless, and it saves an unnecessary system call (not that anybody
> cares about such trivialities, but back in the old days one would have
> done!).
 
Bye

Dominik ^_^

---

From: Julian Bradfield <[email-masked]>
Date: Tue, 8 Jun 1999 12:02:13 +0200 (MDT)

>> Actually, both these statements are wrong, aren't they?
...

>The fix we made disappears once in a while. It's bad news that it didn't

Just to be perfectly clear, since my statement above is a bit
ambigous: fvwm-2.2.2 modules don't do zero-length writes any more, since
this has been fixed in the correct place, namely SendText().
Thus fvwm-2.2.2 is correct, but it still tickles the recent Linux bug.
Making FvwmCommandS.c send " " instead of "" is the sort of workaround
that you were discouraging in your first reply to me, so it shouldn't
be bad news that that isn't in 2.2.2.
So I don't think there's a problem if you hold to the view that other
people's bugs are not your problem.
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 8 Jun 1999 12:48:05 -0600

> people's bugs are not your problem.

It's bad news because 2.2.2 doesn't work and 2.3.3 does. Sending " "
may be a hack, but sending "" is a *bug*. From my Linux man page (write):

  If count is zero and the file descriptor refers  to  a  regular  file,
  0  will  be returned  without causing any other effect.  For a special
  file, the results are not portable.

So think of sending " " as a bug fix since a zero write isn't even defined
for pipes.

You say that FvwmCommandS from 2.2.2 gets a dead pipe but doesn't do zero
length writes. So where does this happen?

Bye

Dominik ^_^