Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 4 Feb 1999 14:19:54 -0800 (PST)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        current@FreeBSD.ORG
Subject:   Bug in piperd
Message-ID:  <199902042219.OAA90785@apollo.backplane.com>

next in thread | raw e-mail | index | archive | help
    Ha.  I've been slowly reducing MAXMEM on my test box to force it to
    swap more heavily running buildworld and found a race in the 
    kern/sys_pipe.c module.

    My buildworld froze up... i.e. just stopped running.  Everything else
    on the box was fine.  ps showed an 'as' command stuck in 'piperd'.

    The problem is simple (pseudo code fragment):

	    pipe read:
		...
		 * check for EOF
		 * check for waiting writers
		 * lock the pipe
		 * check for waiting writers
		 * check for non-blocking I/O
		 * sleep in 'piperd'

    The problem, of course, is that if 'lock the pipe' blocks, it is possible
    for the writer side to close the pipe.  Since EOF is not checked for
    after the pipe has been locked, the reader enters a 'piperd' state
    and never gets woken up again.

    I am testing a fix now and will then commit it.

    If anyone sees anything obviously wrong with this patch, please email me.

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>


Index: sys_pipe.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/sys_pipe.c,v
retrieving revision 1.49
diff -u -r1.49 sys_pipe.c
--- sys_pipe.c	1999/01/28 00:57:47	1.49
+++ sys_pipe.c	1999/02/04 22:14:58
@@ -381,12 +381,32 @@
 #endif
 		} else {
 			/*
+			 * If there is no more to read in the pipe, reset
+			 * its pointers to the beginning.  This improves
+			 * cache hit stats.
+			 *
+			 * We get this over with now because it may block
+			 * and cause the state to change out from under us,
+			 * rather then have to re-test the state both before
+			 * and after this fragment.
+			 */
+		
+			if ((error = pipelock(rpipe,1)) == 0) {
+				if (rpipe->pipe_buffer.cnt == 0) {
+					rpipe->pipe_buffer.in = 0;
+					rpipe->pipe_buffer.out = 0;
+				}
+				pipeunlock(rpipe);
+			}
+
+			/*
 			 * detect EOF condition
 			 */
 			if (rpipe->pipe_state & PIPE_EOF) {
 				/* XXX error = ? */
 				break;
 			}
+
 			/*
 			 * If the "write-side" has been blocked, wake it up now.
 			 */
@@ -394,34 +414,26 @@
 				rpipe->pipe_state &= ~PIPE_WANTW;
 				wakeup(rpipe);
 			}
-			if (nread > 0)
+
+			/*
+			 * break if error (signal via pipelock), or if some 
+			 * data was read
+			 */
+			if (error || nread > 0)
 				break;
 
+			/*
+			 * Handle non-blocking mode operation
+			 */
+
 			if (fp->f_flag & FNONBLOCK) {
 				error = EAGAIN;
 				break;
 			}
 
 			/*
-			 * If there is no more to read in the pipe, reset
-			 * its pointers to the beginning.  This improves
-			 * cache hit stats.
+			 * Wait for more data
 			 */
-		
-			if ((error = pipelock(rpipe,1)) == 0) {
-				if (rpipe->pipe_buffer.cnt == 0) {
-					rpipe->pipe_buffer.in = 0;
-					rpipe->pipe_buffer.out = 0;
-				}
-				pipeunlock(rpipe);
-			} else {
-				break;
-			}
-
-			if (rpipe->pipe_state & PIPE_WANTW) {
-				rpipe->pipe_state &= ~PIPE_WANTW;
-				wakeup(rpipe);
-			}
 
 			rpipe->pipe_state |= PIPE_WANTR;
 			if ((error = tsleep(rpipe, PRIBIO|PCATCH, "piperd", 0)) != 0) {

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199902042219.OAA90785>