pthreads: Add pthread_cleanup_push() and pthread_cleanup_pop()

TODO

@@ -1,4 +1,4 @@
-NuttX TODO List (Last updated December 3, 2016)
+NuttX TODO List (Last updated December 17, 2016)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 This file summarizes known NuttX bugs, limitations, inconsistencies with
@@ -101,7 +101,7 @@ o Task/Scheduler (sched/)
   Status:      Open
   Priority:    Medium Low for now
 
-  Title:       ISSUES WITH atexit() AND on_exit()
+  Title:       ISSUES WITH atexit(), on_exit(), AND pthread_cleanup_pop()
   Description: These functions execute with the following bad properties:
 
                1. They run with interrupts disabled,

include/nuttx/sched.h

@@ -322,6 +322,18 @@ struct child_status_s
 };
 #endif
 
+/* struct pthread_cleanup_s ******************************************************/
+
+#ifdef CONFIG_PTHREAD_CLEANUP
+/* This structure describes one element of the pthread cleanup stack */
+
+struct pthread_cleanup_s
+{
+   pthread_cleanup_t pc_cleaner;    /* Cleanup callback address */
+   FAR void *pc_arg;                /* Argument that accompanies the callback */
+};
+#endif
+
 /* struct dspace_s ***************************************************************/
 /* This structure describes a reference counted D-Space region.  This must be a
  * separately allocated "break-away" structure that can be owned by a task and
@@ -682,6 +694,17 @@ struct pthread_tcb_s
   pthread_addr_t arg;                    /* Startup argument                    */
   FAR void *joininfo;                    /* Detach-able info to support join    */
 
+  /* Clean-up stack *************************************************************/
+
+#ifdef CONFIG_PTHREAD_CLEANUP
+  /* tos   - The index to the next avaiable entry at the top of the stack.
+   * stack - The pre-allocated clean-up stack memory.
+   */
+
+  uint8_t tos;
+  struct pthread_cleanup_s stack[CONFIG_PTHREAD_CLEANUP_STACKSIZE];
+#endif
+
   /* POSIX Thread Specific Data *************************************************/
 
 #if CONFIG_NPTHREAD_KEYS > 0

include/pthread.h

@@ -265,6 +265,12 @@ typedef struct pthread_barrier_s pthread_barrier_t;
 typedef bool pthread_once_t;
 #define __PTHREAD_ONCE_T_DEFINED 1
 
+#ifdef CONFIG_PTHREAD_CLEANUP
+/* This type describes the pthread cleanup callback (non-standard) */
+
+typedef CODE void (*pthread_cleanup_t)(FAR void *arg);
+#endif
+
 /* Forward references */
 
 struct sched_param; /* Defined in sched.h */
@@ -336,6 +342,15 @@ int  pthread_cancel(pthread_t thread);
 int  pthread_setcancelstate(int state, FAR int *oldstate);
 void pthread_testcancel(void);
 
+/* A thread may set up cleanup functions to execut when the thread exits or
+ * is canceled.
+ */
+
+#ifdef CONFIG_PTHREAD_CLEANUP
+void pthread_cleanup_pop(int execute);
+void pthread_cleanup_push(pthread_cleanup_t routine, FAR void *arg);
+#endif
+
 /* A thread can await termination of another thread and retrieve the return
  * value of the thread.
  */

include/sys/syscall.h

@@ -406,7 +406,7 @@
 #  define SYS_pthread_setspecific      (__SYS_pthread+26)
 #  define SYS_pthread_yield            (__SYS_pthread+27)
 
-#  ifndef CONFIG_SMP
+#  ifdef CONFIG_SMP
 #    define SYS_pthread_setaffinity_np (__SYS_pthread+28)
 #    define SYS_pthread_getaffinity_np (__SYS_pthread+29)
 #    define __SYS_pthread_signals      (__SYS_pthread+30)
@@ -418,9 +418,17 @@
 #    define SYS_pthread_cond_timedwait (__SYS_pthread_signals+0)
 #    define SYS_pthread_kill           (__SYS_pthread_signals+1)
 #    define SYS_pthread_sigmask        (__SYS_pthread_signals+2)
-#    define __SYS_mqueue               (__SYS_pthread_signals+3)
+#    define __SYS_pthread_cleanup      (__SYS_pthread_signals+3)
 #  else
-#    define __SYS_mqueue               __SYS_pthread_signals
+#    define __SYS_pthread_cleanup      __SYS_pthread_signals
+#  endif
+
+#  ifdef CONFIG_PTHREAD_CLEANUP
+#    define __SYS_pthread_cleanup_push (__SYS_pthread_cleanup+0)
+#    define __SYS_pthread_cleanup_pop  (__SYS_pthread_cleanup+1)
+#    define __SYS_mqueue               (__SYS_pthread_cleanup+2)
+#  else
+#    define __SYS_mqueue               __SYS_pthread_cleanup
 #  endif
 
 #else

sched/Kconfig

@@ -537,6 +537,24 @@ config NPTHREAD_KEYS
 		The number of items of thread-
 		specific data that can be retained
 
+config PTHREAD_CLEANUP
+	bool "pthread cleanup stack"
+	default n
+	---help---
+		Select to enable support for pthread exit cleanup stacks.  This
+		enables the interfaces pthread_cleanup_push() and
+		pthread_cleanup_pop().
+
+config PTHREAD_CLEANUP_STACKSIZE
+	int "pthread cleanup stack size"
+	default 1
+	range 1 32
+	depends on PTHREAD_CLEANUP
+	---help---
+		The maximum number of cleanup actions that may be pushed by
+		pthread_clean_push().  This setting will increase the size of EVERY
+		pthread task control block by about 8 * CONFIG_PTHREAD_CLEANUP_STACKSIZE.
+
 endmenu # Pthread Options
 
 menu "Performance Monitoring"

sched/pthread/Make.defs

@@ -55,6 +55,10 @@ ifeq ($(CONFIG_SMP),y)
 CSRCS += pthread_setaffinity.c pthread_getaffinity.c
 endif
 
+ifeq ($(CONFIG_PTHREAD_CLEANUP),y)
+CSRCS += pthread_cleanup.c
+endif
+
 # Include pthread build support
 
 DEPPATH += --dep-path pthread

sched/pthread/pthread.h

@@ -96,6 +96,11 @@ struct task_group_s;  /* Forward reference */
 void weak_function pthread_initialize(void);
 int pthread_schedsetup(FAR struct pthread_tcb_s *tcb, int priority, start_t start,
                        pthread_startroutine_t entry);
+
+#ifdef CONFIG_PTHREAD_CLEANUP
+void pthread_cleanup_popall(FAR struct pthread_tcb_s *tcb);
+#endif
+
 int pthread_completejoin(pid_t pid, FAR void *exit_value);
 void pthread_destroyjoin(FAR struct task_group_s *group,
                          FAR struct join_s *pjoin);

sched/pthread/pthread_cancel.c

@@ -53,7 +53,7 @@
 
 int pthread_cancel(pthread_t thread)
 {
-  FAR struct tcb_s *tcb;
+  FAR struct pthread_tcb_s *tcb;
 
   /* First, make sure that the handle references a valid thread */
 
@@ -66,8 +66,8 @@ int pthread_cancel(pthread_t thread)
       return ESRCH;
     }
 
-  tcb = sched_gettcb((pid_t)thread);
+  tcb = (FAR struct pthread_tcb_s *)sched_gettcb((pid_t)thread);
-  if (!tcb)
+  if (tcb == NULL)
     {
       /* The pid does not correspond to any known thread.  The thread
        * has probably already exited.
@@ -76,13 +76,15 @@ int pthread_cancel(pthread_t thread)
       return ESRCH;
     }
 
+  DEBUGASSERT((tcb-cmn.flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD);
+
   /* Check to see if this thread has the non-cancelable bit set in its
    * flags. Suppress context changes for a bit so that the flags are stable.
    * (the flags should not change in interrupt handling.
    */
 
   sched_lock();
-  if ((tcb->flags & TCB_FLAG_NONCANCELABLE) != 0)
+  if ((tcb->cmn.flags & TCB_FLAG_NONCANCELABLE) != 0)
     {
       /* Then we cannot cancel the thread now.  Here is how this is
        * supposed to work:
@@ -97,7 +99,7 @@ int pthread_cancel(pthread_t thread)
        *  processing."
        */
 
-      tcb->flags |= TCB_FLAG_CANCEL_PENDING;
+      tcb->cmn.flags |= TCB_FLAG_CANCEL_PENDING;
       sched_unlock();
       return OK;
     }
@@ -108,11 +110,17 @@ int pthread_cancel(pthread_t thread)
    * same as pthread_exit(PTHREAD_CANCELED).
    */
 
-  if (tcb == this_task())
+  if (tcb == (FAR struct pthread_tcb_s *)this_task())
     {
       pthread_exit(PTHREAD_CANCELED);
     }
 
+#ifdef CONFIG_PTHREAD_CLEANUP
+   /* Perform any stack pthread clean-up callbacks */
+
+   pthread_cleanup_popall(tcb);
+#endif
+
   /* Complete pending join operations */
 
   (void)pthread_completejoin((pid_t)thread, PTHREAD_CANCELED);

sched/pthread/pthread_cleanup.c

@@ -0,0 +1,212 @@
+/****************************************************************************
+ * sched/pthread/pthread_cleanup.c
+ *
+ *   Copyright (C) 2016 Gregory Nutt. All rights reserved.
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <pthread.h>
+
+#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+
+#include "sched/sched.h"
+#include "pthread/pthread.h"
+
+#ifdef CONFIG_PTHREAD_CLEANUP
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pthread_cleanup_pop_tcb
+ *
+ * Description:
+ *   The pthread_cleanup_pop_tcb() function will remove the routine at the top
+ *   of the calling thread's cancellation cleanup stack and optionally
+ *   invoke it (if 'execute' is non-zero).
+ *
+ * Input Parameters:
+ *   tcb - The TCB of the pthread that is exiting or being canceled.
+ *
+ * Return Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static void pthread_cleanup_pop_tcb(FAR struct pthread_tcb_s *tcb, int execute)
+{
+  if (tcb->tos > 0)
+    {
+      unsigned int ndx;
+
+      /* Get the index to the last cleaner function pushed onto the stack */
+
+      ndx = tcb->tos - 1;
+      DEBUGASSERT(ndx < CONFIG_PTHREAD_CLEANUP_STACKSIZE);
+
+      /* Should we execute the cleanup routine at the top of the stack? */
+
+      if (execute != 0)
+        {
+          FAR struct pthread_cleanup_s *cb;
+
+          /* Yes..  Execute the clean-up routine.
+           *
+           * REVISIT: This is a security problem In the PROTECTED and KERNEL
+           * builds:  We must not call the registered function in supervisor
+           * mode!  See also on_exit() and atexit() callbacks.
+           */
+
+          cb  = &tcb->stack[ndx];
+          cb->pc_cleaner(cb->pc_arg);
+        }
+
+      tcb->tos = ndx;
+    }
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pthread_cleanup_push
+ *       pthread_cleanup_pop
+ *
+ * Description:
+ *   The pthread_cleanup_pop() function will remove the routine at the top
+ *   of the calling thread's cancellation cleanup stack and optionally
+ *   invoke it (if 'execute' is non-zero).
+ *
+ *   The pthread_cleanup_push() function will push the specified cancellation
+ *   cleanup handler routine onto the calling thread's cancellation cleanup
+ *   stack. The cancellation cleanup handler will be popped from the
+ *   cancellation cleanup stack and invoked with the argument arg when:
+ *
+ *   - The thread exits (that is, calls pthread_exit()).
+ *   - The thread acts upon a cancellation request.
+ *   - The thread calls pthread_cleanup_pop() with a non-zero execute argument.
+ *
+ * Input Parameters:
+ *   routine - The cleanup routine to be pushed on the the cleanup stack.
+ *   arg     - An argument that will accompany the callback.
+ *   execute - Execute the popped cleanup function immediately.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void pthread_cleanup_pop(int execute)
+{
+  FAR struct pthread_tcb_s *tcb = (FAR struct pthread_tcb_s *)this_task();
+  irqstate_t flags;
+
+  /* We don't assert if called from a non-pthread; we just don't do anything */
+
+  DEBUGASSERT(tcb != NULL);
+
+  flags = enter_critical_section();
+  if ((tcb->cmn.flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD)
+    {
+      pthread_cleanup_pop_tcb(tcb, execute);
+    }
+
+  leave_critical_section(flags);
+}
+
+void pthread_cleanup_push(pthread_cleanup_t routine, FAR void *arg)
+{
+  FAR struct pthread_tcb_s *tcb = (FAR struct pthread_tcb_s *)this_task();
+  irqstate_t flags;
+
+  /* We don't assert if called from a non-pthread; we just don't do anything */
+
+  DEBUGASSERT(tcb != NULL);
+  DEBUGASSERT(tcb->tos < CONFIG_PTHREAD_CLEANUP_STACKSIZE);
+
+  flags = enter_critical_section();
+  if ((tcb->cmn.flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD &&
+       tcb->tos < CONFIG_PTHREAD_CLEANUP_STACKSIZE)
+    {
+      unsigned int ndx = tcb->tos;
+
+      tcb->tos++;
+      tcb->stack[ndx].pc_cleaner = routine;
+      tcb->stack[ndx].pc_arg = arg;
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: pthread_cleanup_popall
+ *
+ * Description:
+ *   The pthread_cleanup_popall() is an internal function that will pop and
+ *   execute all clean-up functions.  This function is only called from within
+ *   the pthread_exit() and pthread_cancellation() logic
+ *
+ * Input Parameters:
+ *   tcb - The TCB of the pthread that is exiting or being canceled.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void pthread_cleanup_popall(FAR struct pthread_tcb_s *tcb)
+{
+  irqstate_t flags;
+
+  DEBUGASSERT(tcb != NULL);
+  DEBUGASSERT((tcb->cmn.flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD);
+
+  /* Pop and execute each cleanup routine */
+
+  flags = enter_critical_section();
+  while (tcb->tos > 0)
+    {
+      pthread_cleanup_pop_tcb(tcb, 1);
+    }
+
+  leave_critical_section(flags);
+}
+
+#endif /* CONFIG_PTHREAD_CLEANUP */

sched/pthread/pthread_exit.c

@@ -80,6 +80,9 @@ void pthread_exit(FAR void *exit_value)
 
   sinfo("exit_value=%p\n", exit_value);
 
+  DEBUGASSERT(tcb != NULL);
+  DEBUGASSERT((tcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD);
+
   /* Block any signal actions that would awaken us while were
    * are performing the JOIN handshake.
    */
@@ -91,6 +94,12 @@ void pthread_exit(FAR void *exit_value)
   }
 #endif
 
+#ifdef CONFIG_PTHREAD_CLEANUP
+   /* Perform any stack pthread clean-up callbacks */
+
+   pthread_cleanup_popall((FAR struct pthread_tcb_s *)tcb);
+#endif
+
   /* Complete pending join operations */
 
   status = pthread_completejoin(getpid(), exit_value);

sched/task/task_exithook.c

@@ -75,7 +75,8 @@ static inline void task_atexit(FAR struct tcb_s *tcb)
    * callbacks.
    *
    * REVISIT: This is a security problem In the PROTECTED and KERNEL builds:
-   * We must not call the registered function in supervisor mode!
+   * We must not call the registered function in supervisor mode!  See also
+   * on_exit() and pthread_cleanup_pop() callbacks.
    */
 
   if (group && group->tg_nmembers == 1)
@@ -138,7 +139,8 @@ static inline void task_onexit(FAR struct tcb_s *tcb, int status)
    * callbacks.
    *
    * REVISIT: This is a security problem In the PROTECTED and KERNEL builds:
-   * We must not call the registered function in supervisor mode!
+   * We must not call the registered function in supervisor mode!  See also
+   * atexit() and pthread_cleanup_pop() callbacks.
    */
 
   if (group && group->tg_nmembers == 1)

syscall/syscall.csv

@@ -64,6 +64,8 @@
 "pthread_barrier_init","pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","FAR pthread_barrier_t*","FAR const pthread_barrierattr_t*","unsigned int"
 "pthread_barrier_wait","pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","FAR pthread_barrier_t*"
 "pthread_cancel","pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","pthread_t"
+"pthread_cleanup_pop","pthread.h","defined(CONFIG_PTHREAD_CLEANUP)","void","int"
+"pthread_cleanup_push","pthread.h","defined(CONFIG_PTHREAD_CLEANUP)","void","pthread_cleanup_t","FAR void*"
 "pthread_cond_broadcast","pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","FAR pthread_cond_t*"
 "pthread_cond_destroy","pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","FAR pthread_cond_t*"
 "pthread_cond_init","pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","FAR pthread_cond_t*","FAR const pthread_condattr_t*"

syscall/syscall_lookup.h

@@ -304,6 +304,10 @@ SYSCALL_LOOKUP(up_assert,                 2, STUB_up_assert)
   SYSCALL_LOOKUP(pthread_kill,            2, STUB_pthread_kill)
   SYSCALL_LOOKUP(pthread_sigmask,         3, STUB_pthread_sigmask)
 #  endif
+#  ifdef CONFIG_PTHREAD_CLEANUP
+  SYSCALL_LOOKUP(pthread_cleanup_push,    2, STUB_pthread_cleanup_push)
+  SYSCALL_LOOKUP(pthread_cleanup_pop,     1, STUB_pthread_cleanup_pop)
+#  endif
 #endif
 
 /* The following are defined only if message queues are enabled */

syscall/syscall_stublookup.c

@@ -311,6 +311,10 @@ uintptr_t STUB_pthread_kill(int nbr, uintptr_t parm1, uintptr_t parm2);
 uintptr_t STUB_pthread_sigmask(int nbr, uintptr_t parm1, uintptr_t parm2,
             uintptr_t parm3);
 
+uintptr_t STUB_pthread_cleanup_pop(int nbr, uintptr_t parm1);
+uintptr_t STUB_pthread_cleanup_push(int nbr, uintptr_t parm1,
+            uintptr_t parm2);
+
 /* The following are defined only if message queues are enabled */
 
 uintptr_t STUB_mq_close(int nbr, uintptr_t parm1);