Sebastian Huber wrote:
+/**
+ * @brief Lock to allow once mutex's to be initialized.
+ */
+POSIX_EXTERN pthread_mutex_t _POSIX_Once_Lock;
The POSIX_EXTERN in combination with the definition in once.c is wrong.
Fixed. Thanks.
+{
+ pthread_mutexattr_t mattr;
+ int r;
I would name this "eno" since the status is encoded as an errno number.
Sure, if you wish.
+
+ _POSIX_Once_Lock = PTHREAD_MUTEX_INITIALIZER;
+
+ r = pthread_mutexattr_init( &mattr );
+ if ( r != 0 )
+ rtems_fatal( RTEMS_FATAL_SOURCE_ASSERT, 0x80aa0000 | r );
Errors in the pthread_mutexattr_init(), pthread_mutexattr_settype(), and
pthread_mutexattr_destroy() are pure programming errors, so I should use
_Assert() here.
I considered this and was mixed about the need to check an error code
returned from a POSIX call or not. I am happy to use _Assert.
+
+ r = pthread_mutexattr_settype( &mattr, PTHREAD_MUTEX_RECURSIVE );
+ if ( r != 0 )
+ rtems_fatal( RTEMS_FATAL_SOURCE_ASSERT, 0x80aa1000 | r );
+
+ r = pthread_mutex_init( &_POSIX_Once_Lock, &mattr );
+ if ( r != 0 )
+ rtems_fatal( RTEMS_FATAL_SOURCE_ASSERT, 0x80aa2000 | r );
This is a configuration error. We need a proper error code for this with
source INTERNAL_ERROR_POSIX_API.
Agree. I will change it to INTERNAL_ERROR_POSIX_API. I am not sure what
you mean by proper error code.
We need also a new test for this fatal
error, e.g. psxtests/psxfatal03.
Ok. I will add one.
+
+ pthread_mutexattr_destroy( &mattr );
Check return value with _Assert()?
Done.
+}
diff --git a/cpukit/posix/src/pthreadonce.c
b/cpukit/posix/src/pthreadonce.c
index 0700a22..2b02f1e 100644
--- a/cpukit/posix/src/pthreadonce.c
+++ b/cpukit/posix/src/pthreadonce.c
@@ -25,25 +25,55 @@
#include <rtems.h>
#include <rtems/system.h>
-#include <rtems/score/thread.h>
+#include <rtems/posix/onceimpl.h>
+
+#define PTHREAD_ONCE_INIT_NOT_RUN 0
+#define PTHREAD_ONCE_INIT_RUNNING 1
+#define PTHREAD_ONCE_INIT_RUN 2
I would rename PTHREAD_ONCE_INIT_RUN to PTHREAD_ONCE_INIT_COMPLETE to
avoid too many run combinations.
Sure.
int pthread_once(
pthread_once_t *once_control,
void (*init_routine)(void)
)
{
+ int r = 0;
+
if ( !once_control || !init_routine )
return EINVAL;
- if ( !once_control->init_executed ) {
- rtems_mode saveMode;
- rtems_task_mode(RTEMS_NO_PREEMPT, RTEMS_PREEMPT_MASK, &saveMode);
- if ( !once_control->init_executed ) {
- once_control->is_initialized = true;
- once_control->init_executed = true;
- (*init_routine)();
+ if ( once_control->is_initialized != 1 )
+ return EINVAL;
+
+ if ( once_control->init_executed != PTHREAD_ONCE_INIT_RUN ) {
+ r = pthread_mutex_lock( &_POSIX_Once_Lock );
You get an error here,
1. a pure programming error, e.g. call of this function before
_POSIX_Once_Manager_initialization(), or
2. a corrupt system.
I would simply use _Assert() here.
I will make it an internal error. An assert does not catch the corrupt
system at runtime.
+ if ( r == 0 ) {
+ int rr;
+
+ /*
+ * Getting to here means the once_control is locked so we have:
+ * 1. The init has not run and the state is PTHREAD_ONCE_INIT_NOT_RUN.
+ * 2. The init has finished and the state is PTHREAD_ONCE_INIT_RUN.
+ * 3. The init is being run by this thread and the state
+ * PTHREAD_ONCE_INIT_RUNNING so we are nesting. This is an error.
+ */
+
+ switch ( once_control->init_executed ) {
+ case PTHREAD_ONCE_INIT_NOT_RUN:
+ once_control->init_executed = PTHREAD_ONCE_INIT_RUNNING;
+ (*init_routine)();
+ once_control->init_executed = PTHREAD_ONCE_INIT_RUN;
+ break;
+ case PTHREAD_ONCE_INIT_RUNNING:
+ r = EINVAL;
+ break;
+ default:
+ break;
+ }
+ rr = pthread_mutex_unlock( &_POSIX_Once_Lock );
+ if ( r == 0 )
+ r = rr;
}
- rtems_task_mode(saveMode, RTEMS_PREEMPT_MASK, &saveMode);
}
- return 0;
+
+ return r;
}
[...]
I think the nesting case is not covered by a test.
I will check.
Thanks
Chris
_______________________________________________
rtems-devel mailing list
rtems-devel@rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel
