[PATCH] posix: Use cleanup contexts on the stack

Sebastian Huber sebastian.huber at embedded-brains.de
Fri Jul 5 14:05:32 UTC 2013


This change must be in synchronization with a Newlib <pthread.h> change.
The cleanup contexts are stored on the thread stack.  This is conform to
the POSIX requirements for the pthread_cleanup_push() and
pthread_cleanup_pop() statement pair.

Passing an invalid pointer as the routine to pthread_cleanup_push() is
now a usage error and the behaviour is undefined.
---
 cpukit/posix/include/rtems/posix/cancel.h    |   12 ------
 cpukit/posix/include/rtems/posix/threadsup.h |    6 ++-
 cpukit/posix/src/cancelrun.c                 |   26 +++++-------
 cpukit/posix/src/cleanuppop.c                |   53 +++++---------------------
 cpukit/posix/src/cleanuppush.c               |   44 +++++++--------------
 cpukit/posix/src/pthread.c                   |    2 +-
 testsuites/psxtests/psxcleanup/psxcleanup.c  |    7 ---
 7 files changed, 40 insertions(+), 110 deletions(-)

diff --git a/cpukit/posix/include/rtems/posix/cancel.h b/cpukit/posix/include/rtems/posix/cancel.h
index c1fff9c..6ad53d6 100644
--- a/cpukit/posix/include/rtems/posix/cancel.h
+++ b/cpukit/posix/include/rtems/posix/cancel.h
@@ -22,18 +22,6 @@
 #include <rtems/posix/threadsup.h>
 
 /**
- * This structure is used to manage the cancelation handlers.
- */
-typedef struct {
-  /** This field is the Chain Node so we can put these on lists. */
-  Chain_Node  Node;
-  /** This field is the cancelation routine. */
-  void      (*routine)( void * );
-  /** This field is the argument to the cancelation routine. */
-  void       *arg;
-}  POSIX_Cancel_Handler_control;
-
-/**
  * @brief POSIX run thread cancelation.
  *
  * This support routine runs through the chain of cancel handlers that
diff --git a/cpukit/posix/include/rtems/posix/threadsup.h b/cpukit/posix/include/rtems/posix/threadsup.h
index 80f64dc..c4d75c8 100644
--- a/cpukit/posix/include/rtems/posix/threadsup.h
+++ b/cpukit/posix/include/rtems/posix/threadsup.h
@@ -76,9 +76,11 @@ typedef struct {
   int                     cancelability_type;
   /** This indicates if a cancelation has been requested. */
   int                     cancelation_requested;
-  /** This is the set of cancelation handlers. */
-  Chain_Control           Cancellation_Handlers;
 
+  /**
+   * @brief LIFO list of cleanup contexts.
+   */
+  struct _pthread_cleanup_context *last_cleanup_context;
 } POSIX_API_Control;
 
 /**
diff --git a/cpukit/posix/src/cancelrun.c b/cpukit/posix/src/cancelrun.c
index 2d73bda..fb3e596 100644
--- a/cpukit/posix/src/cancelrun.c
+++ b/cpukit/posix/src/cancelrun.c
@@ -34,26 +34,22 @@ void _POSIX_Threads_cancel_run(
   Thread_Control *the_thread
 )
 {
-  POSIX_Cancel_Handler_control      *handler;
-  Chain_Control                     *handler_stack;
-  POSIX_API_Control                 *thread_support;
-  ISR_Level                          level;
+  struct _pthread_cleanup_context *context;
+  POSIX_API_Control               *thread_support;
 
-  thread_support = the_thread->API_Extensions[ THREAD_API_POSIX ];
-
-  handler_stack = &thread_support->Cancellation_Handlers;
+  _Thread_Disable_dispatch();
 
+  thread_support = the_thread->API_Extensions[ THREAD_API_POSIX ];
   thread_support->cancelability_state = PTHREAD_CANCEL_DISABLE;
 
-  while ( !_Chain_Is_empty( handler_stack ) ) {
-    _ISR_Disable( level );
-      handler = (POSIX_Cancel_Handler_control *)
-           _Chain_Tail( handler_stack )->previous;
-      _Chain_Extract_unprotected( &handler->Node );
-    _ISR_Enable( level );
+  context = thread_support->last_cleanup_context;
+  thread_support->last_cleanup_context = NULL;
+
+  _Thread_Enable_dispatch();
 
-    (*handler->routine)( handler->arg );
+  while ( context != NULL ) {
+    ( *context->_routine )( context->_arg );
 
-    _Workspace_Free( handler );
+    context = context->_previous;
   }
 }
diff --git a/cpukit/posix/src/cleanuppop.c b/cpukit/posix/src/cleanuppop.c
index bbadec5..78e2c34 100644
--- a/cpukit/posix/src/cleanuppop.c
+++ b/cpukit/posix/src/cleanuppop.c
@@ -15,67 +15,34 @@
  */
 
 #if HAVE_CONFIG_H
-#include "config.h"
+  #include "config.h"
 #endif
 
 #include <pthread.h>
-#include <errno.h>
 
 #include <rtems/system.h>
-#include <rtems/score/chain.h>
-#include <rtems/score/isr.h>
 #include <rtems/score/thread.h>
-#include <rtems/score/wkspace.h>
-#include <rtems/posix/cancel.h>
 #include <rtems/posix/pthread.h>
-#include <rtems/posix/threadsup.h>
 
 /*
  *  18.2.3.1 Establishing Cancellation Handlers, P1003.1c/Draft 10, p. 184
  */
 
-void pthread_cleanup_pop(
-  int    execute
+void _pthread_cleanup_pop(
+  struct _pthread_cleanup_context *context,
+  int                              execute
 )
 {
-  POSIX_Cancel_Handler_control      *handler;
-  POSIX_Cancel_Handler_control      tmp_handler;
-  Chain_Control                     *handler_stack;
-  POSIX_API_Control                 *thread_support;
-  ISR_Level                          level;
+  POSIX_API_Control *thread_support;
 
-  thread_support = _Thread_Executing->API_Extensions[ THREAD_API_POSIX ];
-
-  handler_stack = &thread_support->Cancellation_Handlers;
-
-  /*
-   * We need interrupts disabled to safely check the chain and pull
-   * the last element off.  But we also need dispatching disabled to
-   * ensure that we do not get prempted and deleted while we are holding
-   * memory that needs to be freed.
-   */
+  if ( execute != 0 ) {
+    ( *context->_routine )( context->_arg );
+  }
 
   _Thread_Disable_dispatch();
-  _ISR_Disable( level );
-
-    if ( _Chain_Is_empty( handler_stack ) ) {
-      _Thread_Enable_dispatch();
-      _ISR_Enable( level );
-      return;
-    }
-
-    handler = (POSIX_Cancel_Handler_control *)
-        _Chain_Tail( handler_stack )->previous;
-    _Chain_Extract_unprotected( &handler->Node );
 
-  _ISR_Enable( level );
-
-  tmp_handler = *handler;
-
-  _Workspace_Free( handler );
+  thread_support = _Thread_Executing->API_Extensions[ THREAD_API_POSIX ];
+  thread_support->last_cleanup_context = context->_previous;
 
   _Thread_Enable_dispatch();
-
-  if ( execute )
-    (*tmp_handler.routine)( tmp_handler.arg );
 }
diff --git a/cpukit/posix/src/cleanuppush.c b/cpukit/posix/src/cleanuppush.c
index 4fb82c2..52597e6 100644
--- a/cpukit/posix/src/cleanuppush.c
+++ b/cpukit/posix/src/cleanuppush.c
@@ -15,54 +15,38 @@
  */
 
 #if HAVE_CONFIG_H
-#include "config.h"
+  #include "config.h"
 #endif
 
 #include <pthread.h>
-#include <errno.h>
 
 #include <rtems/system.h>
-#include <rtems/score/chain.h>
-#include <rtems/score/isr.h>
 #include <rtems/score/thread.h>
-#include <rtems/score/wkspace.h>
-#include <rtems/posix/cancel.h>
 #include <rtems/posix/pthread.h>
-#include <rtems/posix/threadsup.h>
 
 /*
  *  18.2.3.1 Establishing Cancellation Handlers, P1003.1c/Draft 10, p. 184
  */
 
-void pthread_cleanup_push(
-  void   (*routine)( void * ),
-  void    *arg
+void _pthread_cleanup_push(
+  struct _pthread_cleanup_context   *context,
+  void                            ( *routine )( void * ),
+  void                              *arg
 )
 {
-  POSIX_Cancel_Handler_control      *handler;
-  Chain_Control                     *handler_stack;
-  POSIX_API_Control                 *thread_support;
+  POSIX_API_Control *thread_support;
 
-  /*
-   *  The POSIX standard does not address what to do when the routine
-   *  is NULL.  It also does not address what happens when we cannot
-   *  allocate memory or anything else bad happens.
-   */
-  if ( !routine )
-    return;
+  context->_routine = routine;
+  context->_arg = arg;
 
-  _Thread_Disable_dispatch();
-  handler = _Workspace_Allocate( sizeof( POSIX_Cancel_Handler_control ) );
-
-  if ( handler ) {
-    thread_support = _Thread_Executing->API_Extensions[ THREAD_API_POSIX ];
+  /* Use an invalid value here */
+  context->_canceltype = 13;
 
-    handler_stack = &thread_support->Cancellation_Handlers;
+  _Thread_Disable_dispatch();
 
-    handler->routine = routine;
-    handler->arg = arg;
+  thread_support = _Thread_Executing->API_Extensions[ THREAD_API_POSIX ];
+  context->_previous = thread_support->last_cleanup_context;
+  thread_support->last_cleanup_context = context;
 
-    _Chain_Append( handler_stack, &handler->Node );
-  }
   _Thread_Enable_dispatch();
 }
diff --git a/cpukit/posix/src/pthread.c b/cpukit/posix/src/pthread.c
index f35a000..fc17549 100644
--- a/cpukit/posix/src/pthread.c
+++ b/cpukit/posix/src/pthread.c
@@ -198,7 +198,7 @@ static bool _POSIX_Threads_Create_extension(
   api->cancelation_requested = 0;
   api->cancelability_state = PTHREAD_CANCEL_ENABLE;
   api->cancelability_type = PTHREAD_CANCEL_DEFERRED;
-  _Chain_Initialize_empty (&api->Cancellation_Handlers);
+  api->last_cleanup_context = NULL;
 
   /*
    *  If the thread is not a posix thread, then all posix signals are blocked
diff --git a/testsuites/psxtests/psxcleanup/psxcleanup.c b/testsuites/psxtests/psxcleanup/psxcleanup.c
index 568e897..123e766 100644
--- a/testsuites/psxtests/psxcleanup/psxcleanup.c
+++ b/testsuites/psxtests/psxcleanup/psxcleanup.c
@@ -250,13 +250,6 @@ void *POSIX_Init(
 
   sleep(1);
 
-  /*************** ERROR CASES  ***************/
-  puts("Call pthread_cleanup_push with NULL handler");
-  pthread_cleanup_push(NULL, NULL);
-
-  puts("Call pthread_cleanup_pop with no push");
-  pthread_cleanup_pop(1);
-
   /*************** END OF TEST *****************/
   puts( "*** END OF POSIX CLEANUP TEST ***\n" );
   rtems_test_exit(0);
-- 
1.7.7




More information about the devel mailing list