Fix pipe/fifo open logic: semaphore wait in open() must abort if a signal is received

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3327 42af7a65-404d-4744-a932-0658087f49c3
This commit is contained in:
patacongo 2011-03-02 00:33:42 +00:00
parent 8e96987058
commit fefe839818
5 changed files with 85 additions and 50 deletions

View File

@ -1499,3 +1499,8 @@
nsh> cat test.txt
This is a test
* drvers/pipes/pipe_common.c: Driver open method eas not returning an EINTR
error when it received a signal. Instead, it just re-started the wait. This
makes it impossible to kill a background pipe operation from NSH.

View File

@ -8,7 +8,7 @@
<tr align="center" bgcolor="#e4e4e4">
<td>
<h1><big><font color="#3c34ec"><i>NuttX RTOS</i></font></big></h1>
<p>Last Updated: February 28, 2011</p>
<p>Last Updated: March 1, 2011</p>
</td>
</tr>
</table>
@ -2102,6 +2102,10 @@ nuttx-5.19 2011-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
nsh> cat test.txt
This is a test
* drvers/pipes/pipe_common.c: Driver open method eas not returning an EINTR
error when it received a signal. Instead, it just re-started the wait. This
makes it impossible to kill a background pipe operation from NSH.
pascal-2.1 2011-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
buildroot-1.10 2011-xx-xx <spudmonkey@racsa.co.cr>

View File

@ -1,7 +1,7 @@
/****************************************************************************
* drivers/pipes/pipe_common.c
*
* Copyright (C) 2008-2009 Gregory Nutt. All rights reserved.
* Copyright (C) 2008-2009, 2011 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <spudmonkey@racsa.co.cr>
*
* Redistribution and use in source and binary forms, with or without
@ -184,6 +184,7 @@ int pipecommon_open(FAR struct file *filep)
struct inode *inode = filep->f_inode;
struct pipe_dev_s *dev = inode->i_private;
int sval;
int ret;
/* Some sanity checking */
#if CONFIG_DEBUG
@ -192,66 +193,87 @@ int pipecommon_open(FAR struct file *filep)
return -EBADF;
}
#endif
/* Make sure that we have exclusive access to the device structure */
/* Make sure that we have exclusive access to the device structure. The
* sem_wait() call should fail only if we are awakened by a signal.
*/
if (sem_wait(&dev->d_bfsem) == 0)
ret = sem_wait(&dev->d_bfsem);
if (ret != OK)
{
/* If this the first reference on the device, then allocate the buffer */
fdbg("sem_wait failed: %d\n", errno);
DEBUGASSERT(errno > 0);
return -errno;
}
if (dev->d_refs == 0)
/* If this the first reference on the device, then allocate the buffer */
if (dev->d_refs == 0)
{
dev->d_buffer = (uint8_t*)malloc(CONFIG_DEV_PIPE_SIZE);
if (!dev->d_buffer)
{
dev->d_buffer = (uint8_t*)malloc(CONFIG_DEV_PIPE_SIZE);
if (!dev->d_buffer)
(void)sem_post(&dev->d_bfsem);
return -ENOMEM;
}
}
/* Increment the reference count on the pipe instance */
dev->d_refs++;
/* If opened for writing, increment the count of writers on on the pipe instance */
if ((filep->f_oflags & O_WROK) != 0)
{
dev->d_nwriters++;
/* If this this is the first writer, then the read semaphore indicates the
* number of readers waiting for the first writer. Wake them all up.
*/
if (dev->d_nwriters == 1)
{
while (sem_getvalue(&dev->d_rdsem, &sval) == 0 && sval < 0)
{
(void)sem_post(&dev->d_bfsem);
return -ENOMEM;
sem_post(&dev->d_rdsem);
}
}
}
/* Increment the reference count on the pipe instance */
/* If opened for read-only, then wait for at least one writer on the pipe */
dev->d_refs++;
sched_lock();
(void)sem_post(&dev->d_bfsem);
if ((filep->f_oflags & O_RDWR) == O_RDONLY && dev->d_nwriters < 1)
{
/* NOTE: d_rdsem is normally used when the read logic waits for more
* data to be written. But until the first writer has opened the
* pipe, the meaning is different: it is used prevent O_RDONLY open
* calls from returning until there is at least one writer on the pipe.
* This is required both by spec and also because it prevents
* subsequent read() calls from returning end-of-file because there is
* no writer on the pipe.
*/
/* If opened for writing, increment the count of writers on on the pipe instance */
if ((filep->f_oflags & O_WROK) != 0)
ret = sem_wait(&dev->d_rdsem);
if (ret != OK)
{
dev->d_nwriters++;
/* If this this is the first writer, then the read semaphore indicates the
* number of readers waiting for the first writer. Wake them all up.
/* The sem_wait() call should fail only if we are awakened by
* a signal.
*/
if (dev->d_nwriters == 1)
{
while (sem_getvalue(&dev->d_rdsem, &sval) == 0 && sval < 0)
{
sem_post(&dev->d_rdsem);
}
}
fdbg("sem_wait failed: %d\n", errno);
DEBUGASSERT(errno > 0);
ret = -errno;
/* Immediately close the pipe that we just opened */
(void)pipecommon_close(filep);
}
}
/* If opened for read-only, then wait for at least one writer on the pipe */
sched_lock();
(void)sem_post(&dev->d_bfsem);
if ((filep->f_oflags & O_RDWR) == O_RDONLY && dev->d_nwriters < 1)
{
/* NOTE: d_rdsem is normally used when the read logic waits for more
* data to be written. But until the first writer has opened the
* pipe, the meaning is different: it is used prevent O_RDONLY open
* calls from returning until there is at least one writer on the pipe.
* This is required both by spec and also because it prevents
* subsequent read() calls from returning end-of-file because there is
* no writer on the pipe.
*/
pipecommon_semtake(&dev->d_rdsem);
}
sched_unlock();
return OK;
}
return ERROR;
sched_unlock();
return ret;
}
/****************************************************************************

View File

@ -360,7 +360,7 @@ const char g_fmtcmdfailed[] = "nsh: %s: %s failed: %d\n";
const char g_fmtcmdoutofmemory[] = "nsh: %s: out of memory\n";
const char g_fmtinternalerror[] = "nsh: %s: Internal error\n";
#ifndef CONFIG_DISABLE_SIGNALS
const char g_fmtsignalrecvd[] = "nsh: %s: Signal received\n";
const char g_fmtsignalrecvd[] = "nsh: %s: Interrupted by signal\n";
#endif
/****************************************************************************

View File

@ -1,7 +1,7 @@
/****************************************************************************
* fs_open.c
*
* Copyright (C) 2007, 2008, 2009 Gregory Nutt. All rights reserved.
* Copyright (C) 2007-2009, 2011 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <spudmonkey@racsa.co.cr>
*
* Redistribution and use in source and binary forms, with or without
@ -69,6 +69,10 @@ int inode_checkflags(FAR struct inode *inode, int oflags)
}
}
/****************************************************************************
* Name: open
****************************************************************************/
int open(const char *path, int oflags, ...)
{
struct filelist *list;
@ -181,7 +185,7 @@ int open(const char *path, int oflags, ...)
errout_with_inode:
inode_release(inode);
errout:
*get_errno_ptr() = ret;
errno = ret;
return ERROR;
}