NEWLIB BUMP was Re: [PATCH 3/3] posix: Use cleanup contexts on the stack
Joel Sherrill
joel.sherrill at OARcorp.com
Fri Nov 29 23:29:58 UTC 2013
I just committed the corresponding change to newlib.
It is time to bump the tools I think and merge this
patch.
AFAIK all cpuset, pthread affinity, and restrict patches
are now committed to newlib.
--joel
On 7/8/2013 8:25 AM, Sebastian Huber wrote:
> 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..aee7f2b 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 = -1;
>
> - 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);
>
--
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherrill at OARcorp.com On-Line Applications Research
Ask me about RTEMS: a free RTOS Huntsville AL 35805
Support Available (256) 722-9985
More information about the devel
mailing list