Re: pthread_once non conformant

[Jonathan Larmour <jifl@jifvik.org>]

Jerome Souquieres wrote:
> Hi,
> 
> I think I've spotted a bug in pthread_once (in ecos v3.0). The current
> implementation does not enforce the part of the specification that
> says: "On return from pthread_once(), it is guaranteed that
> init_routine() has completed".
> 
> Here is a proposal for a patch, I did not post it to ecos-patches
> because I currently don't have a working eCos target to test this.

[Patch at end]

Thanks. I agree there's a problem here, but the fix isn't right - you 
can't leave pthread_mutex locked while running an arbitrary user routine. 
You'll get a  potential deadlock, or at least incorrect operation of the 
pthread layer if anything uses pthread functions before the user function 
terminates (which could be never).

The real solution probably involves linking a condition variable to 
pthread_mutex and having an additional state for once_control to indicate 
"once function being run but not yet complete". Any thread which sees the 
once_control in that state will have to wait on the condvar (which is then 
broadcast on when the once function returns).

Would you be able to make a fix along those lines? Or failing that if you 
submit it to http://bugs.ecos.sourceware.org/ it won't get lost at least.

Jifl

> --- pthread.cxx	2009-01-29 18:47:52.000000000 +0100
> +++ pthread-fixonce.cxx	2009-11-01 21:07:42.639875000 +0100
> @@ -1328,22 +1328,23 @@ externC int pthread_once (pthread_once_t
>      PTHREAD_ENTRY();
> 
>      PTHREAD_CHECK( once_control );
>      PTHREAD_CHECK( init_routine );
> 
> -    pthread_once_t old;
> -
> -    // Do a test and set on the once_control object.
>      pthread_mutex.lock();
> 
> -    old = *once_control;
> -    *once_control = 1;
> +    // If the once_control is zero, call the init_routine().
> +    // the mutex must stay locked during init_routine because
> +    // concurrent threads must be blocked until init_routine has
> +    // done its business
> +    if ( !*once_control )
> +    {
> +        init_routine();
> +        *once_control = 1;
> +    }
> 
>      pthread_mutex.unlock();
> -
> -    // If the once_control was zero, call the init_routine().
> -    if( !old ) init_routine();
> 
>      PTHREAD_RETURN(0);
>  }
> 


-- 
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine