apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sander Striker" <stri...@apache.org>
Subject RE: cvs commit: apr/threadproc/win32 thread.c
Date Sat, 01 Sep 2001 09:18:47 GMT
> rbb         01/09/01 00:29:07
> 
>   Modified:    threadproc/win32 thread.c
>   Log:
>   Avoid a rather nasty bug in the Windows apr_thread_once function.  We
>   basically, check the value before we increment, and after.  By checking
>   before, we take a hit of one if statement, but avoid a kernel call in
>   most cases.
>   Submitted by:	Will Rowe <wrowe@rowe-clan.net>
>   
>   Revision  Changes    Path
>   1.38      +11 -0     apr/threadproc/win32/thread.c
>   
>   Index: thread.c
>   ===================================================================
>   RCS file: /home/cvs/apr/threadproc/win32/thread.c,v
>   retrieving revision 1.37
>   retrieving revision 1.38
>   diff -u -r1.37 -r1.38
>   --- thread.c	2001/09/01 07:10:25	1.37
>   +++ thread.c	2001/09/01 07:29:07	1.38
>   @@ -231,6 +231,17 @@
>    APR_DECLARE(apr_status_t) apr_thread_once(apr_thread_once_t *control,
>                                              void (*func)(void))
>    {
>   +    /* Quick escape hatch, and bug fix.  The InterlockedIncrement
>   +     * call is just incrementing a long int, so it has the potential
>   +     * to wrap.  Very unlikely, but still, that would be an almost
>   +     * impossible bug to hunt.  With this, we might see one or two
>   +     * calls to InterlockedIncrement, but never enough to wrap the
>   +     * long int.  This also saves us a kernel call for most calls to
>   +     * this function.
>   +     */
>   +    if (control->value) {
>   +        return APR_SUCCESS;
>   +    }
>        InterlockedIncrement(&control->value);
>        if (control->value == 1) {
>            func();

This still isn't correct.  Now we have a race condition that
could result in the function never being called instead of once.
This looks better IMHO:

    APR_DECLARE(apr_status_t) apr_thread_once(apr_thread_once_t *control,
                                              void (*func)(void))
    {         
        if (!InterlockedExchange(&control->value, 1))
            func();

        return APR_SUCCESS;
    }

This way we also don't have the wrapping problem.


Sander


Mime
View raw message