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

@@ -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 */
@@ -39,9 +41,9 @@ extern "C" {
 
 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 );
 
 /** @} */
 

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;
 }

cpukit/score/src/once.c

@@ -1,10 +1,29 @@
 /*
- *  COPYRIGHT (c) 1989-1999.
+ * SPDX-License-Identifier: BSD-2-Clause
- *  On-Line Applications Research Corporation (OAR).
  *
- *  The license and distribution terms for this file may be
+ * Copyright (C) 2019 embedded brains GmbH
- *  found in the file LICENSE in this distribution or at
+ * Copyright (C) 2019 Sebastian Huber
- *  http://www.rtems.org/license/LICENSE.
+ *
+ * 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
@@ -12,57 +31,69 @@
 #endif
 
 #include <rtems/score/onceimpl.h>
-#include <rtems/score/apimutex.h>
+#include <rtems/score/threadimpl.h>
+#include <rtems/thread.h>
 
-#include <errno.h>
+#define ONCE_STATE_INIT 0
 
-#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;
+  _Atomic_Fence( ATOMIC_ORDER_ACQUIRE );
 
-  if ( *once_state != ONCE_STATE_COMPLETE ) {
+  if ( RTEMS_PREDICT_FALSE( *once_state != ONCE_STATE_COMPLETE ) ) {
-    _Once_Lock();
+    Thread_Life_state thread_life_state;
 
-    /*
+    thread_life_state = _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 ) {
+    if ( *once_state == ONCE_STATE_INIT ) {
-      case ONCE_STATE_NOT_RUN:
       *once_state = ONCE_STATE_RUNNING;
+      _Once_Unlock( THREAD_LIFE_PROTECTED );
       ( *init_routine )();
+      _Once_Lock();
+      _Atomic_Fence( ATOMIC_ORDER_RELEASE );
       *once_state = ONCE_STATE_COMPLETE;
-        break;
+      rtems_condition_variable_broadcast( &_Once_Information.State );
-      case ONCE_STATE_RUNNING:
+    } else {
-        eno = EINVAL;
+      while ( *once_state != ONCE_STATE_COMPLETE ) {
-        break;
+        rtems_condition_variable_wait(
-      default:
+          &_Once_Information.State,
-        break;
+          &_Once_Information.Mutex
+        );
+      }
     }
 
-    _Once_Unlock();
+    _Once_Unlock( thread_life_state );
   }
 
-  return eno;
+  return 0;
 }
 
-static API_Mutex_Control _Once_Mutex = API_MUTEX_INITIALIZER( "_Once" );
+Thread_Life_state _Once_Lock( void )
-
-void _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 );
 }

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 )
+static pthread_once_t once_b = PTHREAD_ONCE_INIT;
-{
+
-  int status;
+static rtems_id master;
-  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 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( &once_a, routine_a );
-  status = pthread_once( &nesting_once, Test_init_routine_nesting );
+  rtems_test_assert( status == 0 );
-  rtems_test_assert( !status );
+  rtems_test_assert( test_init_routine_call_counter == 3 );
 
   TEST_END();
   rtems_test_exit( 0 );

testsuites/psxtests/psxonce01/psxonce01.scn

@@ -1,11 +1,14 @@
-
+*** BEGIN OF TEST PSXONCE 1 ***
-
+*** TEST VERSION: 5.0.0.e214ff4b636011bd149e3683c89aa982e361fd1c
-*** TEST POSIX ONCE 01 ***
+*** TEST STATE: EXPECTED-PASS
-Init: pthread_once - SUCCESSFUL (init_routine_nesting executes)
+*** TEST BUILD:
-Test_init_routine_nesting: invoked
+*** 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 ***

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
 

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

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.