9
0
Fork 0

Unix domain/FIFOs: Fix a race condition between FIFO buffer operations and the opening and closing of FIFOs which necessary when the FIFOs are used to support Unix domain, datagram sockets. The default policy is the deallocate FIFO buffering when the last client closes the pipe. When when used for datagram communicatinos, packets left in the FIFO will be lost. Some like UDP read-ahead is needed: The buffered data in the FIFO needs to be retained until the reader gets a chance to re-open the FIFO. Added an ioctl (PIPEIOC_POLICY) to control the buffer policy. Default (0) is the legacy behavior; Unix domain datagram logic sets the alternative policy so that the packet data persists after the FIFO is closed.

This commit is contained in:
Gregory Nutt 2015-01-30 11:14:24 -06:00
parent 26555d8d49
commit 958a1b5aa2
9 changed files with 188 additions and 82 deletions

View File

@ -93,7 +93,7 @@ int client_main(int argc, char *argv[])
sockfd = socket(PF_LOCAL, SOCK_DGRAM, 0);
if (sockfd < 0)
{
printf("client socket failure %d\n", errno);
printf("client: ERROR socket failure %d\n", errno);
return 1;
}
@ -127,13 +127,13 @@ int client_main(int argc, char *argv[])
if (nbytes < 0)
{
printf("client: %d. sendto failed: %d\n", offset, errno);
printf("client: %d. ERROR sendto failed: %d\n", offset, errno);
close(sockfd);
return 1;
}
else if (nbytes != SENDSIZE)
{
printf("client: %d. Bad send length: %d Expected: %d\n",
printf("client: %d. ERROR Bad send length: %d Expected: %d\n",
offset, nbytes, SENDSIZE);
close(sockfd);
return 1;

View File

@ -122,7 +122,7 @@ int server_main(int argc, char *argv[])
if (bind(sockfd, (struct sockaddr*)&server, addrlen) < 0)
{
printf("server: bind failure: %d\n", errno);
printf("server: ERROR bind failure: %d\n", errno);
return 1;
}
@ -137,7 +137,7 @@ int server_main(int argc, char *argv[])
if (nbytes < 0)
{
printf("server: %d. recv failed: %d\n", offset, errno);
printf("server: %d. ERROR recv failed: %d\n", offset, errno);
close(sockfd);
return 1;
}
@ -178,7 +178,7 @@ int server_main(int argc, char *argv[])
if (nbytes != SENDSIZE)
{
printf("server: %d. recv size incorrect: %d vs %d\n",
printf("server: %d. ERROR recv size incorrect: %d vs %d\n",
offset, nbytes, SENDSIZE);
close(sockfd);
return 1;
@ -186,13 +186,13 @@ int server_main(int argc, char *argv[])
if (offset < inbuf[0])
{
printf("server: %d. %d packets lost, resetting offset\n",
printf("server: %d. ERROR %d packets lost, resetting offset\n",
offset, inbuf[0] - offset);
offset = inbuf[0];
}
else if (offset > inbuf[0])
{
printf("server: %d. Bad offset in buffer: %d\n",
printf("server: %d. ERROR Bad offset in buffer: %d\n",
offset, inbuf[0]);
close(sockfd);
return 1;
@ -200,7 +200,7 @@ int server_main(int argc, char *argv[])
if (!check_buffer(inbuf))
{
printf("server: %d. Bad buffer contents\n", offset);
printf("server: %d. ERROR Bad buffer contents\n", offset);
close(sockfd);
return 1;
}

View File

@ -73,7 +73,7 @@ static const struct file_operations fifo_fops =
pipecommon_read, /* read */
pipecommon_write, /* write */
0, /* seek */
0 /* ioctl */
pipecommon_ioctl /* ioctl */
#ifndef CONFIG_DISABLE_POLL
, pipecommon_poll /* poll */
#endif

View File

@ -82,7 +82,7 @@ static const struct file_operations pipe_fops =
pipecommon_read, /* read */
pipecommon_write, /* write */
0, /* seek */
0 /* ioctl */
pipecommon_ioctl /* ioctl */
#ifndef CONFIG_DISABLE_POLL
, pipecommon_poll /* poll */
#endif

View File

@ -1,7 +1,7 @@
/****************************************************************************
* drivers/pipes/pipe_common.c
*
* Copyright (C) 2008-2009, 2011 Gregory Nutt. All rights reserved.
* Copyright (C) 2008-2009, 2011, 2015 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -41,6 +41,7 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <stdint.h>
#include <stdbool.h>
#include <stdlib.h>
@ -188,13 +189,7 @@ int pipecommon_open(FAR struct file *filep)
int sval;
int ret;
/* Some sanity checking */
#if CONFIG_DEBUG
if (!dev)
{
return -EBADF;
}
#endif
DEBUGASSERT(dev);
/* 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.
@ -208,9 +203,12 @@ int pipecommon_open(FAR struct file *filep)
return -get_errno();
}
/* If this the first reference on the device, then allocate the buffer */
/* If this the first reference on the device, then allocate the buffer. In the
* case of d_policy == 1, the buffer already be present when the pipe is
* first opened.
*/
if (dev->d_refs == 0)
if (dev->d_refs == 0 && dev->d_buffer == NULL)
{
dev->d_buffer = (uint8_t*)kmm_malloc(CONFIG_DEV_PIPE_SIZE);
if (!dev->d_buffer)
@ -243,11 +241,17 @@ int pipecommon_open(FAR struct file *filep)
}
}
/* If opened for read-only, then wait for at least one writer on the pipe */
/* If opened for read-only, then wait for either (1) at least one writer
* on the pipe (policy == 0), or (2) until there is buffered data to be
* read (policy == 1).
*/
sched_lock();
(void)sem_post(&dev->d_bfsem);
if ((filep->f_oflags & O_RDWR) == O_RDONLY && dev->d_nwriters < 1)
if ((filep->f_oflags & O_RDWR) == O_RDONLY && /* Read-only */
dev->d_nwriters < 1 && /* No writers on the pipe */
dev->d_wrndx == dev->d_rdndx) /* Buffer is empty */
{
/* 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
@ -289,13 +293,7 @@ int pipecommon_close(FAR struct file *filep)
struct pipe_dev_s *dev = inode->i_private;
int sval;
/* Some sanity checking */
#if CONFIG_DEBUG
if (!dev)
{
return -EBADF;
}
#endif
DEBUGASSERT(dev && dev->d_refs > 0);
/* Make sure that we have exclusive access to the device structure.
* NOTE: close() is supposed to return EINTR if interrupted, however
@ -304,15 +302,17 @@ int pipecommon_close(FAR struct file *filep)
pipecommon_semtake(&dev->d_bfsem);
/* Decrement the number of references on the pipe. Check if there are
* still oustanding references to the pipe.
*/
/* Check if the decremented reference count would go to zero */
if (dev->d_refs > 1)
if (--dev->d_refs > 0)
{
/* No.. then just decrement the reference count */
dev->d_refs--;
/* If opened for writing, decrement the count of writers on on the pipe instance */
/* No more references.. If opened for writing, decrement the count of
* writers on the pipe instance.
*/
if ((filep->f_oflags & O_WROK) != 0)
{
@ -329,9 +329,16 @@ int pipecommon_close(FAR struct file *filep)
}
}
}
else
/* What is the buffer management policy? Do we free the buffe when the
* last client closes the pipe (d_policy == 0), or when the buffer becomes
* empty (d_policy). In the latter case, the buffer data will remain
* valid and can be obtained when the pipe is re-opened.
*/
else if (dev->d_policy == 0 || dev->d_wrndx == dev->d_rdndx)
{
/* Yes... deallocate the buffer */
/* Policy 0 or the buffer is empty ... deallocate the buffer now. */
kmm_free(dev->d_buffer);
dev->d_buffer = NULL;
@ -363,13 +370,7 @@ ssize_t pipecommon_read(FAR struct file *filep, FAR char *buffer, size_t len)
int sval;
int ret;
/* Some sanity checking */
#if CONFIG_DEBUG
if (!dev)
{
return -ENODEV;
}
#endif
DEBUGASSERT(dev);
/* Make sure that we have exclusive access to the device structure */
@ -453,15 +454,7 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer, size_t
int nxtwrndx;
int sval;
/* Some sanity checking */
#if CONFIG_DEBUG
if (!dev)
{
return -ENODEV;
}
#endif
DEBUGASSERT(dev);
pipe_dumpbuffer("To PIPE:", (uint8_t*)buffer, len);
/* At present, this method cannot be called from interrupt handlers. That is
@ -581,14 +574,7 @@ int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
int ret = OK;
int i;
/* Some sanity checking */
#if CONFIG_DEBUG
if (!dev || !fds)
{
return -ENODEV;
}
#endif
DEBUGASSERT(dev && fds);
/* Are we setting up the poll? Or tearing it down? */
@ -679,4 +665,23 @@ errout:
}
#endif
/****************************************************************************
* Name: pipecommon_ioctl
****************************************************************************/
int pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
{
FAR struct inode *inode = filep->f_inode;
FAR struct pipe_dev_s *dev = inode->i_private;
/* Only one command supported */
if (cmd == PIPEIOC_POLICY)
{
dev->d_policy = (arg != 0);
return OK;
}
return -ENOTTY;
}
#endif /* CONFIG_DEV_PIPE_SIZE > 0 */

View File

@ -96,6 +96,7 @@ struct pipe_dev_s
uint8_t d_refs; /* References counts on pipe (limited to 255) */
uint8_t d_nwriters; /* Number of reference counts for write access */
uint8_t d_pipeno; /* Pipe minor number */
bool d_policy; /* Buffer policy: 0=free on close; 1=free on empty */
uint8_t *d_buffer; /* Buffer allocated when device opened */
/* The following is a list if poll structures of threads waiting for
@ -119,14 +120,16 @@ extern "C" {
# define EXTERN extern
#endif
EXTERN FAR struct pipe_dev_s *pipecommon_allocdev(void);
EXTERN void pipecommon_freedev(FAR struct pipe_dev_s *dev);
EXTERN int pipecommon_open(FAR struct file *filep);
EXTERN int pipecommon_close(FAR struct file *filep);
EXTERN ssize_t pipecommon_read(FAR struct file *, FAR char *, size_t);
EXTERN ssize_t pipecommon_write(FAR struct file *, FAR const char *, size_t);
FAR struct pipe_dev_s *pipecommon_allocdev(void);
void pipecommon_freedev(FAR struct pipe_dev_s *dev);
int pipecommon_open(FAR struct file *filep);
int pipecommon_close(FAR struct file *filep);
ssize_t pipecommon_read(FAR struct file *, FAR char *, size_t);
ssize_t pipecommon_write(FAR struct file *, FAR const char *, size_t);
int pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
#ifndef CONFIG_DISABLE_POLL
EXTERN int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
bool setup);
#endif

View File

@ -73,6 +73,7 @@
#define _TCIOCBASE (0x1400) /* Timer ioctl commands */
#define _DJOYBASE (0x1500) /* Discrete joystick ioctl commands */
#define _AJOYBASE (0x1600) /* Analog joystick ioctl commands */
#define _PIPEBASE (0x1700) /* FIFO/pipe ioctl commands */
/* Macros used to manage ioctl commands */
@ -316,6 +317,18 @@
#define _AJOYIOCVALID(c) (_IOC_TYPE(c)==_AJOYBASE)
#define _AJOYIOC(nr) _IOC(_AJOYBASE,nr)
/* FIFOs and pipe driver ioctl definitions **********************************/
#define _PIPEIOCVALID(c) (_IOC_TYPE(c)==_PIPEBASE)
#define _PIPEIOC(nr) _IOC(_PIPEBASE,nr)
#define PIPEIOC_POLICY _PIPEIOC(0x0001) /* Set buffer policy
* IN: unsigned long integer
* 0=free on last close
* (default)
* 1=fre when empty
* OUT: None */
/****************************************************************************
* Public Type Definitions
****************************************************************************/

View File

@ -41,6 +41,8 @@
#if defined(CONFIG_NET) && defined(CONFIG_NET_LOCAL)
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <stdbool.h>
#include <stdio.h>
#include <unistd.h>
@ -49,6 +51,7 @@
#include <errno.h>
#include <assert.h>
#include "local/local.h"
/****************************************************************************
@ -185,8 +188,6 @@ static int local_create_fifo(FAR const char *path)
static int local_release_fifo(FAR const char *path)
{
int ret;
/* Unlink the client-to-server FIFO if it exists. */
if (local_fifo_exists(path))
@ -197,6 +198,8 @@ static int local_release_fifo(FAR const char *path)
*/
#warning Missing logic
#if 0
int ret;
ret = unlink(path);
if (ret < 0)
{
@ -222,8 +225,7 @@ static int local_release_fifo(FAR const char *path)
*
****************************************************************************/
static inline int local_rx_open(FAR struct local_conn_s *conn,
FAR const char *path)
static int local_rx_open(FAR struct local_conn_s *conn, FAR const char *path)
{
conn->lc_infd = open(path, O_RDONLY);
if (conn->lc_infd < 0)
@ -256,8 +258,7 @@ static inline int local_rx_open(FAR struct local_conn_s *conn,
*
****************************************************************************/
static inline int local_tx_open(FAR struct local_conn_s *conn,
FAR const char *path)
static int local_tx_open(FAR struct local_conn_s *conn, FAR const char *path)
{
conn->lc_outfd = open(path, O_WRONLY);
if (conn->lc_outfd < 0)
@ -282,6 +283,36 @@ static inline int local_tx_open(FAR struct local_conn_s *conn,
return OK;
}
/****************************************************************************
* Name: local_set_policy
*
* Description:
* Set the FIFO buffer policy:
*
* 0=Free FIFO resources when the last reference is closed
* 1=Free FIFO resources when the buffer is empty.
*
****************************************************************************/
static int local_set_policy(int fd, unsigned long policy)
{
int ret;
/* Set the buffer policy */
ret = ioctl(fd, PIPEIOC_POLICY, policy);
if (ret < 0)
{
int errcode = get_errno();
DEBUGASSERT(errcode > 0);
ndbg("ERROR: Failed to set FIFO buffer policty: %d\n", errcode);
return -errcode;
}
return OK;
}
/****************************************************************************
* Public Functions
****************************************************************************/
@ -390,6 +421,7 @@ int local_release_halfduplex(FAR struct local_conn_s *conn)
int local_open_client_rx(FAR struct local_conn_s *client)
{
char path[LOCAL_FULLPATH_LEN];
int ret;
/* Get the server-to-client path name */
@ -397,7 +429,15 @@ int local_open_client_rx(FAR struct local_conn_s *client)
/* Then open the file for read-only access */
return local_rx_open(client, path);
ret = local_rx_open(client, path);
if (ret == OK)
{
/* Policy: Free FIFO resources when the last reference is closed */
ret = local_set_policy(client->lc_infd, 0);
}
return ret;
}
/****************************************************************************
@ -411,6 +451,7 @@ int local_open_client_rx(FAR struct local_conn_s *client)
int local_open_client_tx(FAR struct local_conn_s *client)
{
char path[LOCAL_FULLPATH_LEN];
int ret;
/* Get the client-to-server path name */
@ -418,7 +459,15 @@ int local_open_client_tx(FAR struct local_conn_s *client)
/* Then open the file for write-only access */
return local_tx_open(client, path);
ret = local_tx_open(client, path);
if (ret == OK)
{
/* Policy: Free FIFO resources when the last reference is closed */
ret = local_set_policy(client->lc_outfd, 0);
}
return ret;
}
/****************************************************************************
@ -432,6 +481,7 @@ int local_open_client_tx(FAR struct local_conn_s *client)
int local_open_server_rx(FAR struct local_conn_s *server)
{
char path[LOCAL_FULLPATH_LEN];
int ret;
/* Get the client-to-server path name */
@ -439,7 +489,15 @@ int local_open_server_rx(FAR struct local_conn_s *server)
/* Then open the file for write-only access */
return local_rx_open(server, path);
ret = local_rx_open(server, path);
if (ret == OK)
{
/* Policy: Free FIFO resources when the last reference is closed */
ret = local_set_policy(server->lc_infd, 0);
}
return ret;
}
/****************************************************************************
@ -453,6 +511,7 @@ int local_open_server_rx(FAR struct local_conn_s *server)
int local_open_server_tx(FAR struct local_conn_s *server)
{
char path[LOCAL_FULLPATH_LEN];
int ret;
/* Get the server-to-client path name */
@ -460,7 +519,15 @@ int local_open_server_tx(FAR struct local_conn_s *server)
/* Then open the file for read-only access */
return local_tx_open(server, path);
ret = local_tx_open(server, path);
if (ret == OK)
{
/* Policy: Free FIFO resources when the last reference is closed */
ret = local_set_policy(server->lc_outfd, 0);
}
return ret;
}
/****************************************************************************
@ -474,6 +541,7 @@ int local_open_server_tx(FAR struct local_conn_s *server)
int local_open_receiver(FAR struct local_conn_s *conn)
{
char path[LOCAL_FULLPATH_LEN];
int ret;
/* Get the server-to-client path name */
@ -481,7 +549,15 @@ int local_open_receiver(FAR struct local_conn_s *conn)
/* Then open the file for read-only access */
return local_rx_open(conn, path);
ret = local_rx_open(conn, path);
if (ret == OK)
{
/* Policy: Free FIFO resources when the buffer is empty. */
ret = local_set_policy(conn->lc_infd, 1);
}
return ret;
}
/****************************************************************************
@ -495,6 +571,7 @@ int local_open_receiver(FAR struct local_conn_s *conn)
int local_open_sender(FAR struct local_conn_s *conn, FAR const char *path)
{
char fullpath[LOCAL_FULLPATH_LEN];
int ret;
/* Get the server-to-client path name */
@ -502,7 +579,15 @@ int local_open_sender(FAR struct local_conn_s *conn, FAR const char *path)
/* Then open the file for read-only access */
return local_tx_open(conn, fullpath);
ret = local_tx_open(conn, fullpath);
if (ret == OK)
{
/* Policy: Free FIFO resources when the buffer is empty. */
ret = local_set_policy(conn->lc_outfd, 1);
}
return ret;
}
#endif /* CONFIG_NET && CONFIG_NET_LOCAL */

View File

@ -337,7 +337,7 @@ psock_dgram_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len,
/* Adjust the number of bytes remaining to be read from the packet */
DEBUGASSERT(tmplen <= remain);
DEBUGASSERT(tmplen <= remaining);
remaining -= tmplen;
}
while (remaining > 0);