[rtems commit] score: Avoid some deadlocks in _Once()
Sebastian Huber
sebh at rtems.org
Mon Feb 18 06:26:37 UTC 2019
Module: rtems
Branch: master
Commit: e4ad14cc789090290550e3aa9640e4969037e8b7
Changeset: http://git.rtems.org/rtems/commit/?id=e4ad14cc789090290550e3aa9640e4969037e8b7
Author: Sebastian Huber <sebastian.huber at embedded-brains.de>
Date: Tue Feb 12 12:19:38 2019 +0100
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.
---
cpukit/include/rtems/score/onceimpl.h | 10 ++-
cpukit/rtems/src/timerserver.c | 5 +-
cpukit/score/src/once.c | 117 ++++++++++++++++++----------
testsuites/psxtests/psxonce01/init.c | 82 ++++++++++++++++---
testsuites/psxtests/psxonce01/psxonce01.scn | 15 ++--
testsuites/psxtests/psxonce01/system.h | 2 +-
testsuites/sptests/spcxx01/init.cc | 21 ++++-
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 60f1378..f3afe1c 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 09e792a..55c3d96 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 5237c11..2b2cc0e 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 1c90769..41ff5b1 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 2c5d47d..a7afa15 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 10d611b..fe95285 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 7962810..b7be220 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 669e682..73e6a3f 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.
More information about the vc
mailing list