Skip to content

Commit

Permalink
drivers/pipes: using rmutex to protect pipe and avoid deadlock
Browse files Browse the repository at this point in the history 
nxsem_wait
nuttx/sched/semaphore/sem_wait.c:176
nxmutex_lock
nuttx/libs/libc/misc/lib_mutex.c:204 (discriminator 2)
pipecommon_write
nuttx/drivers/pipes/pipe_common.c:538 (discriminator 2)
file_write
nuttx/fs/vfs/fs_write.c:91
write
nuttx/include/unistd.h:523 (discriminator 2)
nxsig_deliver
nuttx/sched/signal/sig_deliver.c:170 (discriminator 4)
arm_sigdeliver
nuttx/arch/arm/src/armv7-a/arm_sigdeliver.c:107
irq_waitlock
nuttx/sched/irq/irq_csection.c:204
nxsem_post
nuttx/sched/semaphore/sem_post.c:86 (discriminator 2)
nxmutex_unlock
nuttx/libs/libc/misc/lib_mutex.c:339 (discriminator 2)
pipecommon_poll
nuttx/drivers/pipes/pipe_common.c:769
file_poll
nuttx/fs/vfs/fs_poll.c:321
poll_fdsetup
nuttx/fs/vfs/fs_poll.c:194
poll
nuttx/include/sys/poll.h:164
uv_run
apps/system/libuv/libuv/src/unix/core.c:449
adb_hal_run
apps/system/adb/microADB/hal/hal_uv.c:76
adbd_main
apps/system/adb/adb_main.c:157
nxtask_startup
nuttx/libs/libc/sched/task_startup.c:70 (discriminator 2)
nxtask_start
nuttx/sched/task/task_start.c:134

Signed-off-by: dongjiuzhu1 <[email protected]>
  • Loading branch information
@Donny9 @xiaoxiang781216
Donny9 authored and xiaoxiang781216 committed Sep 28, 2024
1 parent 786dabf commit 0fb1dc2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 38 deletions.
74 changes: 37 additions & 37 deletions drivers/pipes/pipe_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ FAR struct pipe_dev_s *pipecommon_allocdev(size_t bufsize)
{
/* Initialize the private structure */

nxmutex_init(&dev->d_bflock);
nxrmutex_init(&dev->d_bflock);
nxsem_init(&dev->d_rdsem, 0, 0);
nxsem_init(&dev->d_wrsem, 0, 0);
dev->d_bufsize = bufsize;
Expand All @@ -115,7 +115,7 @@ FAR struct pipe_dev_s *pipecommon_allocdev(size_t bufsize)

void pipecommon_freedev(FAR struct pipe_dev_s *dev)
{
nxmutex_destroy(&dev->d_bflock);
nxrmutex_destroy(&dev->d_bflock);
nxsem_destroy(&dev->d_rdsem);
nxsem_destroy(&dev->d_wrsem);
kmm_free(dev);
Expand All @@ -134,14 +134,14 @@ int pipecommon_open(FAR struct file *filep)
DEBUGASSERT(dev != NULL);

/* Make sure that we have exclusive access to the device structure. The
* nxmutex_lock() call should fail if we are awakened by a signal or if the
* thread was canceled.
* nxrmutex_lock() call should fail if we are awakened by a signal or if
* the thread was canceled.
*/

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
ferr("ERROR: nxmutex_lock failed: %d\n", ret);
ferr("ERROR: nxrmutex_lock failed: %d\n", ret);
return ret;
}

Expand All @@ -152,7 +152,7 @@ int pipecommon_open(FAR struct file *filep)
ret = circbuf_init(&dev->d_buffer, NULL, dev->d_bufsize);
if (ret < 0)
{
nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return ret;
}
}
Expand Down Expand Up @@ -187,7 +187,7 @@ int pipecommon_open(FAR struct file *filep)
* on the pipe.
*/

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);

/* NOTE: d_wrsem is normally used to check if the write buffer is full
* and wait for it being read and being able to receive more data. But,
Expand All @@ -207,14 +207,14 @@ int pipecommon_open(FAR struct file *filep)
return ret;
}

/* The nxmutex_lock() call should fail if we are awakened by a
/* The nxrmutex_lock() call should fail if we are awakened by a
* signal or if the task is canceled.
*/

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
ferr("ERROR: nxmutex_lock failed: %d\n", ret);
ferr("ERROR: nxrmutex_lock failed: %d\n", ret);

/* Immediately close the pipe that we just opened */

Expand Down Expand Up @@ -251,7 +251,7 @@ int pipecommon_open(FAR struct file *filep)
* on the pipe.
*/

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);

/* 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
Expand All @@ -273,14 +273,14 @@ int pipecommon_open(FAR struct file *filep)
return ret;
}

/* The nxmutex_lock() call should fail if we are awakened by a
/* The nxrmutex_lock() call should fail if we are awakened by a
* signal or if the task is canceled.
*/

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
ferr("ERROR: nxmutex_lock failed: %d\n", ret);
ferr("ERROR: nxrmutex_lock failed: %d\n", ret);

/* Immediately close the pipe that we just opened */

Expand All @@ -289,7 +289,7 @@ int pipecommon_open(FAR struct file *filep)
}
}

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return ret;
}

Expand All @@ -310,7 +310,7 @@ int pipecommon_close(FAR struct file *filep)
* I've never seen anyone check that.
*/

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
/* The close will not be performed if the task was canceled */
Expand Down Expand Up @@ -398,7 +398,7 @@ int pipecommon_close(FAR struct file *filep)
#endif
}

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return OK;
}

Expand All @@ -422,7 +422,7 @@ ssize_t pipecommon_read(FAR struct file *filep, FAR char *buffer, size_t len)

/* Make sure that we have exclusive access to the device structure */

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
/* May fail because a signal was received or if the task was
Expand All @@ -440,24 +440,24 @@ ssize_t pipecommon_read(FAR struct file *filep, FAR char *buffer, size_t len)

if (dev->d_nwriters <= 0 && PIPE_IS_POLICY_0(dev->d_flags))
{
nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return 0;
}

/* If O_NONBLOCK was set, then return EGAIN */

if (filep->f_oflags & O_NONBLOCK)
{
nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return -EAGAIN;
}

/* Otherwise, wait for something to be written to the pipe */

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
ret = nxsem_wait(&dev->d_rdsem);

if (ret < 0 || (ret = nxmutex_lock(&dev->d_bflock)) < 0)
if (ret < 0 || (ret = nxrmutex_lock(&dev->d_bflock)) < 0)
{
/* May fail because a signal was received or if the task was
* canceled.
Expand Down Expand Up @@ -488,7 +488,7 @@ ssize_t pipecommon_read(FAR struct file *filep, FAR char *buffer, size_t len)

pipecommon_wakeup(&dev->d_wrsem);

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
pipe_dumpbuffer("From PIPE:", buffer, nread);
return nread;
}
Expand Down Expand Up @@ -517,7 +517,7 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer,
}

/* At present, this method cannot be called from interrupt handlers. That
* is because it calls nxmutex_lock() and nxmutex_lock() cannot be called
* is because it calls nxrmutex_lock() and nxrmutex_lock() cannot be called
* form interrupt level. This actually happens fairly commonly
* IF [a-z]err() is called from interrupt handlers and stdout is being
* redirected via a pipe. In that case, the debug output will try to go
Expand All @@ -533,7 +533,7 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer,

/* Make sure that we have exclusive access to the device structure */

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
/* May fail because a signal was received or if the task was
Expand All @@ -556,7 +556,7 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer,

if (dev->d_nreaders <= 0 && PIPE_IS_POLICY_0(dev->d_flags))
{
nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return nwritten == 0 ? -EPIPE : nwritten;
}

Expand Down Expand Up @@ -589,7 +589,7 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer,

/* Return the number of bytes written */

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return len;
}
}
Expand Down Expand Up @@ -627,17 +627,17 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer,
nwritten = -EAGAIN;
}

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return nwritten;
}

/* There is more to be written.. wait for data to be removed from
* the pipe
*/

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
ret = nxsem_wait(&dev->d_wrsem);
if (ret < 0 || (ret = nxmutex_lock(&dev->d_bflock)) < 0)
if (ret < 0 || (ret = nxrmutex_lock(&dev->d_bflock)) < 0)
{
/* Either call nxsem_wait may fail because a signal was
* received or if the task was canceled.
Expand Down Expand Up @@ -667,7 +667,7 @@ int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,

/* Are we setting up the poll? Or tearing it down? */

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
return ret;
Expand Down Expand Up @@ -764,7 +764,7 @@ int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
}

errout:
nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return ret;
}

Expand All @@ -787,7 +787,7 @@ int pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
}
#endif

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
return ret;
Expand Down Expand Up @@ -902,7 +902,7 @@ int pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
break;
}

nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);
return ret;
}

Expand All @@ -919,7 +919,7 @@ int pipecommon_unlink(FAR struct inode *inode)
DEBUGASSERT(inode->i_private);
dev = inode->i_private;

ret = nxmutex_lock(&dev->d_bflock);
ret = nxrmutex_lock(&dev->d_bflock);
if (ret < 0)
{
return ret;
Expand All @@ -935,7 +935,7 @@ int pipecommon_unlink(FAR struct inode *inode)
/* Mark the pipe unlinked */

PIPE_UNLINK(dev->d_flags);
nxmutex_unlock(&dev->d_bflock);
nxrmutex_unlock(&dev->d_bflock);

return OK;
}
Expand Down
2 changes: 1 addition & 1 deletion drivers/pipes/pipe_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ typedef uint8_t pipe_ndx_t; /* 8-bit index */

struct pipe_dev_s
{
mutex_t d_bflock; /* Used to serialize access to d_buffer and indices */
rmutex_t d_bflock; /* Used to serialize access to d_buffer and indices */
sem_t d_rdsem; /* Empty buffer - Reader waits for data write AND
* block O_RDONLY open until there is at least one writer */
sem_t d_wrsem; /* Full buffer - Writer waits for data read AND
Expand Down

0 comments on commit 0fb1dc2

Please sign in to comment.