diff options
author | Sebastian Huber <sebastian.huber@embedded-brains.de> | 2019-02-12 12:19:38 +0100 |
---|---|---|
committer | Sebastian Huber <sebastian.huber@embedded-brains.de> | 2019-02-18 07:25:58 +0100 |
commit | e4ad14cc789090290550e3aa9640e4969037e8b7 (patch) | |
tree | 94d366cf331ee1f5dc10fc4e4298a41ed878acfd | |
parent | libdl/rap: Add the section alloc call after section load was split (diff) | |
download | rtems-e4ad14cc789090290550e3aa9640e4969037e8b7.tar.bz2 |
score: Avoid some deadlocks in _Once()
Recursive usage of the same pthread_once_t results now in a deadlock.
Previously, an error of EINVAL was returned. This usage scenario is
invalid according to the POSIX pthread_once() specification.
Close #3334.
-rw-r--r-- | cpukit/include/rtems/score/onceimpl.h | 10 | ||||
-rw-r--r-- | cpukit/rtems/src/timerserver.c | 5 | ||||
-rw-r--r-- | cpukit/score/src/once.c | 117 | ||||
-rw-r--r-- | testsuites/psxtests/psxonce01/init.c | 82 | ||||
-rw-r--r-- | testsuites/psxtests/psxonce01/psxonce01.scn | 15 | ||||
-rw-r--r-- | testsuites/psxtests/psxonce01/system.h | 2 | ||||
-rw-r--r-- | testsuites/sptests/spcxx01/init.cc | 21 | ||||
-rw-r--r-- | testsuites/sptests/spcxx01/spcxx01.doc | 2 |
8 files changed, 183 insertions, 71 deletions
diff --git a/cpukit/include/rtems/score/onceimpl.h b/cpukit/include/rtems/score/onceimpl.h index 60f1378506..f3afe1cd13 100644 --- a/cpukit/include/rtems/score/onceimpl.h +++ b/cpukit/include/rtems/score/onceimpl.h @@ -7,7 +7,7 @@ */ /* - * Copyright (c) 2014 embedded brains GmbH. All rights reserved. + * Copyright (c) 2014, 2019 embedded brains GmbH. All rights reserved. * * embedded brains GmbH * Dornierstr. 4 @@ -23,6 +23,8 @@ #ifndef _RTEMS_ONCE_H #define _RTEMS_ONCE_H +#include <rtems/score/thread.h> + #ifdef __cplusplus extern "C" { #endif /* __cplusplus */ @@ -37,11 +39,11 @@ extern "C" { * @{ */ -int _Once( unsigned char *once_state, void (*init_routine)(void) ); +int _Once( unsigned char *once_state, void ( *init_routine )( void ) ); -void _Once_Lock( void ); +Thread_Life_state _Once_Lock( void ); -void _Once_Unlock( void ); +void _Once_Unlock( Thread_Life_state thread_life_state ); /** @} */ diff --git a/cpukit/rtems/src/timerserver.c b/cpukit/rtems/src/timerserver.c index 09e792aa1c..55c3d96b9b 100644 --- a/cpukit/rtems/src/timerserver.c +++ b/cpukit/rtems/src/timerserver.c @@ -228,10 +228,11 @@ rtems_status_code rtems_timer_initiate_server( ) { rtems_status_code status; + Thread_Life_state thread_life_state; - _Once_Lock(); + thread_life_state = _Once_Lock(); status = _Timer_server_Initiate( priority, stack_size, attribute_set ); - _Once_Unlock(); + _Once_Unlock( thread_life_state ); return status; } diff --git a/cpukit/score/src/once.c b/cpukit/score/src/once.c index 5237c11878..2b2cc0e89b 100644 --- a/cpukit/score/src/once.c +++ b/cpukit/score/src/once.c @@ -1,68 +1,99 @@ /* - * COPYRIGHT (c) 1989-1999. - * On-Line Applications Research Corporation (OAR). + * SPDX-License-Identifier: BSD-2-Clause * - * The license and distribution terms for this file may be - * found in the file LICENSE in this distribution or at - * http://www.rtems.org/license/LICENSE. + * Copyright (C) 2019 embedded brains GmbH + * Copyright (C) 2019 Sebastian Huber + * + * 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. + * + * 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. */ #ifdef HAVE_CONFIG_H - #include "config.h" +#include "config.h" #endif #include <rtems/score/onceimpl.h> -#include <rtems/score/apimutex.h> +#include <rtems/score/threadimpl.h> +#include <rtems/thread.h> + +#define ONCE_STATE_INIT 0 -#include <errno.h> +#define ONCE_STATE_RUNNING 1 -#define ONCE_STATE_NOT_RUN 0 -#define ONCE_STATE_RUNNING 1 #define ONCE_STATE_COMPLETE 2 +typedef struct { + rtems_mutex Mutex; + rtems_condition_variable State; +} Once_Control; + +static Once_Control _Once_Information = { + .Mutex = RTEMS_MUTEX_INITIALIZER( "_Once" ), + .State = RTEMS_CONDITION_VARIABLE_INITIALIZER( "_Once" ) +}; + int _Once( unsigned char *once_state, void ( *init_routine )( void ) ) { - int eno = 0; - - if ( *once_state != ONCE_STATE_COMPLETE ) { - _Once_Lock(); - - /* - * Getting to here means the once_control is locked so we have: - * 1. The init has not run and the state is ONCE_STATE_NOT_RUN. - * 2. The init has finished and the state is ONCE_STATE_COMPLETE (already - * caught by the previous if). - * 3. The init is being run by this thread and the state - * ONCE_STATE_RUNNING so we are nesting. This is an error. - */ - - switch ( *once_state ) { - case ONCE_STATE_NOT_RUN: - *once_state = ONCE_STATE_RUNNING; - ( *init_routine )(); - *once_state = ONCE_STATE_COMPLETE; - break; - case ONCE_STATE_RUNNING: - eno = EINVAL; - break; - default: - break; + _Atomic_Fence( ATOMIC_ORDER_ACQUIRE ); + + if ( RTEMS_PREDICT_FALSE( *once_state != ONCE_STATE_COMPLETE ) ) { + Thread_Life_state thread_life_state; + + thread_life_state = _Once_Lock(); + + if ( *once_state == ONCE_STATE_INIT ) { + *once_state = ONCE_STATE_RUNNING; + _Once_Unlock( THREAD_LIFE_PROTECTED ); + ( *init_routine )(); + _Once_Lock(); + _Atomic_Fence( ATOMIC_ORDER_RELEASE ); + *once_state = ONCE_STATE_COMPLETE; + rtems_condition_variable_broadcast( &_Once_Information.State ); + } else { + while ( *once_state != ONCE_STATE_COMPLETE ) { + rtems_condition_variable_wait( + &_Once_Information.State, + &_Once_Information.Mutex + ); + } } - _Once_Unlock(); + _Once_Unlock( thread_life_state ); } - return eno; + return 0; } -static API_Mutex_Control _Once_Mutex = API_MUTEX_INITIALIZER( "_Once" ); - -void _Once_Lock( void ) +Thread_Life_state _Once_Lock( void ) { - _API_Mutex_Lock( &_Once_Mutex ); + Thread_Life_state thread_life_state; + + thread_life_state = _Thread_Set_life_protection( THREAD_LIFE_PROTECTED ); + rtems_mutex_lock( &_Once_Information.Mutex ); + + return thread_life_state; } -void _Once_Unlock( void ) +void _Once_Unlock( Thread_Life_state thread_life_state ) { - _API_Mutex_Unlock( &_Once_Mutex ); + rtems_mutex_unlock( &_Once_Information.Mutex ); + _Thread_Set_life_protection( thread_life_state ); } diff --git a/testsuites/psxtests/psxonce01/init.c b/testsuites/psxtests/psxonce01/init.c index 1c90769d38..41ff5b146b 100644 --- a/testsuites/psxtests/psxonce01/init.c +++ b/testsuites/psxtests/psxonce01/init.c @@ -1,4 +1,7 @@ /* + * Copyright (C) 2019 embedded brains GmbH + * Copyright (C) 2019 Sebastian Huber + * * COPYRIGHT (c) 1989-2009. * On-Line Applications Research Corporation (OAR). * @@ -16,16 +19,11 @@ const char rtems_test_name[] = "PSXONCE 1"; -static pthread_once_t nesting_once = PTHREAD_ONCE_INIT; +static pthread_once_t once_a = PTHREAD_ONCE_INIT; -static void Test_init_routine_nesting( void ) -{ - int status; - puts( "Test_init_routine_nesting: invoked" ); - puts( "Test_init_routine_nesting: pthread_once - EINVAL (init_routine_nesting does not execute)" ); - status = pthread_once( &nesting_once, Test_init_routine_nesting ); - rtems_test_assert( status == EINVAL ); -} +static pthread_once_t once_b = PTHREAD_ONCE_INIT; + +static rtems_id master; static int test_init_routine_call_counter = 0; @@ -35,6 +33,66 @@ static void Test_init_routine( void ) ++test_init_routine_call_counter; } +static void routine_b( void ) +{ + rtems_status_code sc; + + rtems_test_assert( test_init_routine_call_counter == 2 ); + ++test_init_routine_call_counter; + + sc = rtems_event_send( master, RTEMS_EVENT_0 ); + rtems_test_assert( sc == RTEMS_SUCCESSFUL ); +} + +static void use_b( rtems_task_argument arg ) +{ + int status; + + (void) arg; + + status = pthread_once( &once_b, routine_b ); + rtems_test_assert( status == 0 ); + + rtems_task_exit(); +} + +static void routine_a( void ) +{ + rtems_status_code sc; + rtems_id id; + rtems_event_set events; + + rtems_test_assert( test_init_routine_call_counter == 1 ); + ++test_init_routine_call_counter; + + master = rtems_task_self(); + + sc = rtems_task_create( + rtems_build_name( 'T', 'A', 'S', 'K' ), + RTEMS_MINIMUM_PRIORITY, + RTEMS_MINIMUM_STACK_SIZE, + RTEMS_DEFAULT_MODES, + RTEMS_DEFAULT_ATTRIBUTES, + &id + ); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_start( id, use_b, 0 ); + assert( sc == RTEMS_SUCCESSFUL ); + + events = 0; + sc = rtems_event_receive( + RTEMS_EVENT_0, + RTEMS_EVENT_ANY | RTEMS_WAIT, + RTEMS_NO_TIMEOUT, + &events + ); + rtems_test_assert( sc == RTEMS_SUCCESSFUL ); + rtems_test_assert( events == RTEMS_EVENT_0 ); + + rtems_test_assert( test_init_routine_call_counter == 3 ); +} + rtems_task Init(rtems_task_argument argument) { int status; @@ -62,9 +120,9 @@ rtems_task Init(rtems_task_argument argument) printf( "Init: call counter: %d\n", test_init_routine_call_counter ); rtems_test_assert( test_init_routine_call_counter == 1 ); - puts( "Init: pthread_once - SUCCESSFUL (init_routine_nesting executes)" ); - status = pthread_once( &nesting_once, Test_init_routine_nesting ); - rtems_test_assert( !status ); + status = pthread_once( &once_a, routine_a ); + rtems_test_assert( status == 0 ); + rtems_test_assert( test_init_routine_call_counter == 3 ); TEST_END(); rtems_test_exit( 0 ); diff --git a/testsuites/psxtests/psxonce01/psxonce01.scn b/testsuites/psxtests/psxonce01/psxonce01.scn index 2c5d47d2d1..a7afa154cb 100644 --- a/testsuites/psxtests/psxonce01/psxonce01.scn +++ b/testsuites/psxtests/psxonce01/psxonce01.scn @@ -1,11 +1,14 @@ - - -*** TEST POSIX ONCE 01 *** -Init: pthread_once - SUCCESSFUL (init_routine_nesting executes) -Test_init_routine_nesting: invoked +*** BEGIN OF TEST PSXONCE 1 *** +*** TEST VERSION: 5.0.0.e214ff4b636011bd149e3683c89aa982e361fd1c +*** TEST STATE: EXPECTED-PASS +*** TEST BUILD: +*** TEST TOOLS: 7.4.0 20181206 (RTEMS 5, RSB c41b9d0df7e5b4a5056ca50c2534380a44e92769, Newlib 3e24fbf6f) Init: pthread_once - EINVAL (NULL once_control) Init: pthread_once - EINVAL (NULL init_routine) Init: pthread_once - SUCCESSFUL (init_routine executes) Test_init_routine: invoked +Init: call counter: 1 Init: pthread_once - SUCCESSFUL (init_routine does not execute) -*** END OF TEST POSIX ONCE 01 *** +Init: call counter: 1 + +*** END OF TEST PSXONCE 1 *** diff --git a/testsuites/psxtests/psxonce01/system.h b/testsuites/psxtests/psxonce01/system.h index 10d611b86d..fe95285d5f 100644 --- a/testsuites/psxtests/psxonce01/system.h +++ b/testsuites/psxtests/psxonce01/system.h @@ -21,7 +21,7 @@ #define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION -#define CONFIGURE_MAXIMUM_TASKS 1 +#define CONFIGURE_MAXIMUM_TASKS 2 #define CONFIGURE_RTEMS_INIT_TASKS_TABLE diff --git a/testsuites/sptests/spcxx01/init.cc b/testsuites/sptests/spcxx01/init.cc index 7962810da0..b7be220092 100644 --- a/testsuites/sptests/spcxx01/init.cc +++ b/testsuites/sptests/spcxx01/init.cc @@ -29,6 +29,7 @@ #include "config.h" #endif +#include <future> #include <new> #include <rtems.h> @@ -41,10 +42,8 @@ struct alignas(256) S { int i; }; -extern "C" void Init(rtems_task_argument arg) +static void test_aligned_new(void) { - TEST_BEGIN(); - int *i = static_cast<decltype(i)>( ::operator new(sizeof(*i), std::align_val_t(256)) ); @@ -58,6 +57,20 @@ extern "C" void Init(rtems_task_argument arg) rtems_test_assert(s != nullptr); rtems_test_assert(reinterpret_cast<uintptr_t>(s) % 256 == 0); delete s; +} + +static void test_future(void) +{ + std::future<int> f = std::async(std::launch::async, []{ return 12358; }); + rtems_test_assert(f.get() == 12358); +} + +extern "C" void Init(rtems_task_argument arg) +{ + TEST_BEGIN(); + + test_aligned_new(); + test_future(); TEST_END(); exit(0); @@ -69,6 +82,8 @@ extern "C" void Init(rtems_task_argument arg) #define CONFIGURE_MAXIMUM_TASKS 1 +#define CONFIGURE_MAXIMUM_POSIX_THREADS 1 + #define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION #define CONFIGURE_RTEMS_INIT_TASKS_TABLE diff --git a/testsuites/sptests/spcxx01/spcxx01.doc b/testsuites/sptests/spcxx01/spcxx01.doc index 669e6820ab..73e6a3f83a 100644 --- a/testsuites/sptests/spcxx01/spcxx01.doc +++ b/testsuites/sptests/spcxx01/spcxx01.doc @@ -5,7 +5,9 @@ test set name: spcxx01 directives: - ::operator new() + - std::async() concepts: - Ensure that the aligned new operator works. + - Ensure that std::async() works. |