From 7fce8022c6c84cfddd556f2387910a2d8677652b Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Fri, 9 Dec 2016 16:50:34 -0600 Subject: [PATCH] Finishes all cancellation point logic --- TODO | 40 ++++++++++++++++++++++++++- libc/aio/aio_suspend.c | 21 -------------- sched/mqueue/mq_receive.c | 7 +++++ sched/mqueue/mq_send.c | 7 +++++ sched/mqueue/mq_timedreceive.c | 10 +++++++ sched/mqueue/mq_timedsend.c | 10 +++++++ sched/pthread/pthread_condtimedwait.c | 6 ++++ sched/pthread/pthread_condwait.c | 9 +++++- sched/pthread/pthread_join.c | 12 +++++++- sched/semaphore/sem_timedwait.c | 7 +++++ sched/signal/sig_nanosleep.c | 7 +++++ sched/signal/sig_pause.c | 15 ++++++++-- sched/signal/sig_suspend.c | 8 +++++- sched/signal/sig_timedwait.c | 5 ++++ sched/signal/sig_waitinfo.c | 15 +++++++++- 15 files changed, 150 insertions(+), 29 deletions(-) diff --git a/TODO b/TODO index 62839abb7b..802c68b2e8 100644 --- a/TODO +++ b/TODO @@ -14,7 +14,7 @@ nuttx/: (1) Memory Management (mm/) (1) Power Management (drivers/pm) (3) Signals (sched/signal, arch/) - (1) pthreads (sched/pthread) + (2) pthreads (sched/pthread) (0) Message Queues (sched/mqueue) (8) Kernel/Protected Build (3) C++ Support @@ -590,6 +590,44 @@ o pthreads (sched/pthreads) solution. So I discarded a few hours of programming. Not a big loss from the experience I gained." + Title: ISSUES WITH CANCELLATION POINTS + Description: According to POIX cancellation points must occur when a thread is executing + the following functions. There are some execptions as noted: + + accept() mq_timedsend() NA putpmsg() sigtimedwait() + -4 aio_suspend() NA msgrcv() pwrite() NA sigwait() + NA clock_nanosleep() NA msgsnd() read() sigwaitinfo() + close() NA msync() NA readv() -1 sleep() + connect() nanosleep() recv() -2 system() + OK creat() open() recvfrom() NA tcdrain() + fcntl() pause() NA recvmsg() -1 usleep() + NA fdatasync() poll() select() OK wait() + fsync() pread() sem_timedwait() waitid() + NA getmsg() NA pselect() sem_wait() waitpid() + NA getpmsg() pthread_cond_timedwait() send() write() + NA lockf() pthread_cond_wait() NA sendmsg() NA writev() + mq_receive() pthread_join() sendto() + mq_send() pthread_testcancel() -3 sigpause() + mq_timedreceive() NA putmsg() sigsuspend() + + NA Not supported + OK Doesn't need instrumentation. Handled by lower level calls. + -n See note n + + NOTE 1: sleep() and usleep() are user-space functions in the C library and cannot + serve as cancellation points. They are, however, simple wrappers around nanosleep + which is a true cancellation point. + NOTE 2: system() is actually implemented in apps/ as part of NSH. It cannot be + a cancellation point either. + NOTE 3: sigpause() is a user-space function in the C library and cannot serve as + cancellation points. It is, however, a simple wrapper around sigsuspend() + which is a true cancellation point. + NOTE 4: aio_suspend() is a user-space function in the C library and cannot serve as + cancellation points. It does call around sigtimedwait() which is a true cancellation + point. + Status: Not really open. This is just the way it is. + Priority: Nothing additional is planned. + o Message Queues (sched/mqueue) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/libc/aio/aio_suspend.c b/libc/aio/aio_suspend.c index 948462cd90..34b7e4a72b 100644 --- a/libc/aio/aio_suspend.c +++ b/libc/aio/aio_suspend.c @@ -47,27 +47,6 @@ #ifdef CONFIG_FS_AIO -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ -/* Configuration ************************************************************/ - -/**************************************************************************** - * Private Types - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Public Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ diff --git a/sched/mqueue/mq_receive.c b/sched/mqueue/mq_receive.c index 4fed3b5afd..a7fa225541 100644 --- a/sched/mqueue/mq_receive.c +++ b/sched/mqueue/mq_receive.c @@ -47,6 +47,7 @@ #include #include +#include #include "mqueue/mqueue.h" @@ -103,12 +104,17 @@ ssize_t mq_receive(mqd_t mqdes, FAR char *msg, size_t msglen, DEBUGASSERT(up_interrupt_context() == false); + /* mq_receive() is a cancellation point */ + + enter_cancellation_point(); + /* Verify the input parameters and, in case of an error, set * errno appropriately. */ if (mq_verifyreceive(mqdes, msg, msglen) != OK) { + leave_cancellation_point(); return ERROR; } @@ -145,5 +151,6 @@ ssize_t mq_receive(mqd_t mqdes, FAR char *msg, size_t msglen, } sched_unlock(); + leave_cancellation_point(); return ret; } diff --git a/sched/mqueue/mq_send.c b/sched/mqueue/mq_send.c index 12ce2b85c1..045a57eca1 100644 --- a/sched/mqueue/mq_send.c +++ b/sched/mqueue/mq_send.c @@ -46,6 +46,7 @@ #include #include +#include #include "mqueue/mqueue.h" @@ -103,12 +104,17 @@ int mq_send(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio) irqstate_t flags; int ret = ERROR; + /* mq_send() is a cancellation point */ + + enter_cancellation_point(); + /* Verify the input parameters -- setting errno appropriately * on any failures to verify. */ if (mq_verifysend(mqdes, msg, msglen, prio) != OK) { + leave_cancellation_point(); return ERROR; } @@ -177,5 +183,6 @@ int mq_send(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio) } sched_unlock(); + leave_cancellation_point(); return ret; } diff --git a/sched/mqueue/mq_timedreceive.c b/sched/mqueue/mq_timedreceive.c index b5bb100b89..7b97d65294 100644 --- a/sched/mqueue/mq_timedreceive.c +++ b/sched/mqueue/mq_timedreceive.c @@ -50,6 +50,7 @@ #include #include #include +#include #include "sched/sched.h" #include "clock/clock.h" @@ -174,18 +175,24 @@ ssize_t mq_timedreceive(mqd_t mqdes, FAR char *msg, size_t msglen, DEBUGASSERT(up_interrupt_context() == false && rtcb->waitdog == NULL); + /* mq_timedreceive() is a cancellation point */ + + enter_cancellation_point(); + /* Verify the input parameters and, in case of an error, set * errno appropriately. */ if (mq_verifyreceive(mqdes, msg, msglen) != OK) { + leave_cancellation_point(); return ERROR; } if (!abstime || abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) { set_errno(EINVAL); + leave_cancellation_point(); return ERROR; } @@ -198,6 +205,7 @@ ssize_t mq_timedreceive(mqd_t mqdes, FAR char *msg, size_t msglen, if (!rtcb->waitdog) { set_errno(EINVAL); + leave_cancellation_point(); return ERROR; } @@ -250,6 +258,7 @@ ssize_t mq_timedreceive(mqd_t mqdes, FAR char *msg, size_t msglen, rtcb->waitdog = NULL; set_errno(result); + leave_cancellation_point(); return ERROR; } @@ -288,5 +297,6 @@ ssize_t mq_timedreceive(mqd_t mqdes, FAR char *msg, size_t msglen, sched_unlock(); wd_delete(rtcb->waitdog); rtcb->waitdog = NULL; + leave_cancellation_point(); return ret; } diff --git a/sched/mqueue/mq_timedsend.c b/sched/mqueue/mq_timedsend.c index 0b82fb8edb..ccd18c69a1 100644 --- a/sched/mqueue/mq_timedsend.c +++ b/sched/mqueue/mq_timedsend.c @@ -50,6 +50,7 @@ #include #include #include +#include #include "clock/clock.h" #include "sched/sched.h" @@ -178,6 +179,10 @@ int mq_timedsend(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio, DEBUGASSERT(up_interrupt_context() == false && rtcb->waitdog == NULL); + /* mq_timedsend() is a cancellation point */ + + enter_cancellation_point(); + /* Verify the input parameters -- setting errno appropriately * on any failures to verify. */ @@ -186,6 +191,7 @@ int mq_timedsend(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio, { /* mq_verifysend() will set the errno appropriately */ + leave_cancellation_point(); return ERROR; } @@ -199,6 +205,7 @@ int mq_timedsend(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio, */ set_errno(ENOMEM); + leave_cancellation_point(); return ERROR; } @@ -229,6 +236,7 @@ int mq_timedsend(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio, ret = mq_dosend(mqdes, mqmsg, msg, msglen, prio); sched_unlock(); + leave_cancellation_point(); return ret; } @@ -320,6 +328,7 @@ int mq_timedsend(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio, sched_unlock(); wd_delete(rtcb->waitdog); rtcb->waitdog = NULL; + leave_cancellation_point(); return ret; /* Exit here with (1) the scheduler locked, (2) a message allocated, (3) a @@ -341,5 +350,6 @@ errout_with_mqmsg: sched_unlock(); set_errno(result); + leave_cancellation_point(); return ERROR; } diff --git a/sched/pthread/pthread_condtimedwait.c b/sched/pthread/pthread_condtimedwait.c index 7950c5b5fc..d9756a6217 100644 --- a/sched/pthread/pthread_condtimedwait.c +++ b/sched/pthread/pthread_condtimedwait.c @@ -51,6 +51,7 @@ #include #include +#include #include "sched/sched.h" #include "pthread/pthread.h" @@ -178,6 +179,10 @@ int pthread_cond_timedwait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex, DEBUGASSERT(rtcb->waitdog == NULL); + /* pthread_cond_timedwait() is a cancellation point */ + + enter_cancellation_point(); + /* Make sure that non-NULL references were provided. */ if (!cond || !mutex) @@ -338,6 +343,7 @@ int pthread_cond_timedwait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex, } } + leave_cancellation_point(); sinfo("Returning %d\n", ret); return ret; } diff --git a/sched/pthread/pthread_condwait.c b/sched/pthread/pthread_condwait.c index a9ad2ba91c..65eadfc131 100644 --- a/sched/pthread/pthread_condwait.c +++ b/sched/pthread/pthread_condwait.c @@ -1,7 +1,7 @@ /**************************************************************************** * sched/pthread/pthread_condwait.c * - * Copyright (C) 2007-2009, 2012 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2009, 2012, 2016 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -45,6 +45,8 @@ #include #include +#include + #include "pthread/pthread.h" /**************************************************************************** @@ -73,6 +75,10 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex) sinfo("cond=0x%p mutex=0x%p\n", cond, mutex); + /* pthread_cond_wait() is a cancellation point */ + + enter_cancellation_point(); + /* Make sure that non-NULL references were provided. */ if (!cond || !mutex) @@ -111,6 +117,7 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex) } } + leave_cancellation_point(); sinfo("Returning %d\n", ret); return ret; } diff --git a/sched/pthread/pthread_join.c b/sched/pthread/pthread_join.c index 9b73fd4268..2ea88ee8f8 100644 --- a/sched/pthread/pthread_join.c +++ b/sched/pthread/pthread_join.c @@ -1,7 +1,7 @@ /**************************************************************************** * sched/pthread/pthread_join.c * - * Copyright (C) 2007, 2008, 2011, 2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008, 2011, 2013, 2016 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -37,12 +37,16 @@ * Included Files ****************************************************************************/ +#include + #include #include #include #include #include +#include + #include "sched/sched.h" #include "group/group.h" #include "pthread/pthread.h" @@ -90,12 +94,17 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value) sinfo("thread=%d group=%p\n", thread, group); DEBUGASSERT(group); + /* pthread_join() is a cancellation point */ + + enter_cancellation_point(); + /* First make sure that this is not an attempt to join to * ourself. */ if ((pid_t)thread == getpid()) { + leave_cancellation_point(); return EDEADLK; } @@ -230,6 +239,7 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value) ret = OK; } + leave_cancellation_point(); sinfo("Returning %d\n", ret); return ret; } diff --git a/sched/semaphore/sem_timedwait.c b/sched/semaphore/sem_timedwait.c index 81e7a79d50..26cfa4db99 100644 --- a/sched/semaphore/sem_timedwait.c +++ b/sched/semaphore/sem_timedwait.c @@ -49,6 +49,7 @@ #include #include #include +#include #include "sched/sched.h" #include "clock/clock.h" @@ -103,6 +104,10 @@ int sem_timedwait(FAR sem_t *sem, FAR const struct timespec *abstime) DEBUGASSERT(up_interrupt_context() == false && rtcb->waitdog == NULL); + /* sem_timedwait() is a cancellation point */ + + enter_cancellation_point(); + /* Verify the input parameters and, in case of an error, set * errno appropriately. */ @@ -209,6 +214,7 @@ success_with_irqdisabled: leave_critical_section(flags); wd_delete(rtcb->waitdog); rtcb->waitdog = NULL; + leave_cancellation_point(); return OK; /* Error exits */ @@ -220,5 +226,6 @@ errout_with_irqdisabled: errout: set_errno(errcode); + leave_cancellation_point(); return ERROR; } diff --git a/sched/signal/sig_nanosleep.c b/sched/signal/sig_nanosleep.c index 4a492f0c85..e37d9142b0 100644 --- a/sched/signal/sig_nanosleep.c +++ b/sched/signal/sig_nanosleep.c @@ -46,6 +46,7 @@ #include #include +#include #include "clock/clock.h" @@ -112,6 +113,10 @@ int nanosleep(FAR const struct timespec *rqtp, FAR struct timespec *rmtp) int ret; #endif + /* nanosleep() is a cancellation point */ + + enter_cancellation_point(); + if (!rqtp || rqtp->tv_nsec < 0 || rqtp->tv_nsec >= 1000000000) { errval = EINVAL; @@ -154,6 +159,7 @@ int nanosleep(FAR const struct timespec *rqtp, FAR struct timespec *rmtp) /* The timeout "error" is the normal, successful result */ leave_critical_section(flags); + leave_cancellation_point(); return OK; } @@ -203,5 +209,6 @@ int nanosleep(FAR const struct timespec *rqtp, FAR struct timespec *rmtp) errout: set_errno(errval); + leave_cancellation_point(); return ERROR; } diff --git a/sched/signal/sig_pause.c b/sched/signal/sig_pause.c index 80fc712cb7..1aa48b6004 100644 --- a/sched/signal/sig_pause.c +++ b/sched/signal/sig_pause.c @@ -1,7 +1,7 @@ /**************************************************************************** * sched/signal/sig_pause.c * - * Copyright (C) 2012 Gregory Nutt. All rights reserved. + * Copyright (C) 2012, 2016 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -42,6 +42,8 @@ #include #include +#include + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -72,8 +74,13 @@ int pause(void) { - sigset_t set; struct siginfo value; + sigset_t set; + int ret; + + /* pause() is a cancellation point */ + + enter_cancellation_point(); /* Set up for the sleep. Using the empty set means that we are not * waiting for any particular signal. However, any unmasked signal @@ -86,5 +93,7 @@ int pause(void) * meaning that some unblocked signal was caught. */ - return sigwaitinfo(&set, &value); + ret = sigwaitinfo(&set, &value); + leave_cancellation_point(); + return ret; } diff --git a/sched/signal/sig_suspend.c b/sched/signal/sig_suspend.c index d3a4a9ba71..a717d76ce2 100644 --- a/sched/signal/sig_suspend.c +++ b/sched/signal/sig_suspend.c @@ -46,6 +46,7 @@ #include #include +#include #include "sched/sched.h" #include "signal/signal.h" @@ -98,6 +99,10 @@ int sigsuspend(FAR const sigset_t *set) irqstate_t flags; int unblocksigno; + /* sigsuspend() is a cancellation point */ + + enter_cancellation_point(); + /* Several operations must be performed below: We must determine if any * signal is pending and, if not, wait for the signal. Since signals can * be posted from the interrupt level, there is a race condition that @@ -154,5 +159,6 @@ int sigsuspend(FAR const sigset_t *set) } sched_unlock(); - return ERROR; + leave_cancellation_point(); + return ERROR; /* ??REVISIT: Always returns ERROR?? */ } diff --git a/sched/signal/sig_timedwait.c b/sched/signal/sig_timedwait.c index 687884dcf9..9ac4062beb 100644 --- a/sched/signal/sig_timedwait.c +++ b/sched/signal/sig_timedwait.c @@ -52,6 +52,7 @@ #include #include #include +#include #include "sched/sched.h" #include "signal/signal.h" @@ -173,6 +174,9 @@ int sigtimedwait(FAR const sigset_t *set, FAR struct siginfo *info, DEBUGASSERT(rtcb->waitdog == NULL); + /* sigtimedwait() is a cancellation point */ + + enter_cancellation_point(); sched_lock(); /* Not necessary */ /* Several operations must be performed below: We must determine if any @@ -343,5 +347,6 @@ int sigtimedwait(FAR const sigset_t *set, FAR struct siginfo *info, } sched_unlock(); + leave_cancellation_point(); return ret; } diff --git a/sched/signal/sig_waitinfo.c b/sched/signal/sig_waitinfo.c index d271dc9884..470624c338 100644 --- a/sched/signal/sig_waitinfo.c +++ b/sched/signal/sig_waitinfo.c @@ -38,8 +38,11 @@ ****************************************************************************/ #include + #include +#include + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -65,5 +68,15 @@ int sigwaitinfo(FAR const sigset_t *set, FAR struct siginfo *info) { - return sigtimedwait(set, info, NULL); + int ret; + + /* sigwaitinfo() is a cancellation point */ + + enter_cancellation_point(); + + /* Just a wrapper around sigtimedwait() */ + + ret = sigtimedwait(set, info, NULL); + leave_cancellation_point(); + return ret; }