> 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 <[EMAIL PROTECTED]>
>
> 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
