[PATCH] apr win32 thread_cond

[C K Tan]

I ran into a race condition on Win32 with the attached test program (pc.c). Looking at thread_cond.c, it became obvious where where this could happen:

1. on line 96, apr_thread_mutex_unlock can be called multiple times without corresponding lock operation (if the while loop executes more than once).
2. on line 101 & 115, the ReleaseMutex calls do not have the prior lock mutex.

I have not fixed apr_thread_cond_broadcast() which has the same bugs.

Also, apr_thread_cond_create does not register thread_cond_cleanup as was done by the unix version.

Regards,
-cktan

patch

Description: Binary data
/*
 *  Copyright (c) Nx Scientific Limited. All rights reserved.
 *
 *  $Id: pc.c,v 1.1 2003/10/28 05:16:10 cktan Exp $
 */
#include "apr_thread_proc.h"
#include "apr_thread_mutex.h"
#include "apr_thread_cond.h"
#include <stdio.h>


struct {
    int max;
    apr_thread_mutex_t* mutex;
    apr_thread_cond_t* not_full;
    apr_thread_cond_t* not_empty;
    int pcnt, ccnt;
    int token;
} cb = {0};



void* APR_THREAD_FUNC consumer(apr_thread_t* tp, void* _data)
{
    int e;
    static int s_id = 0;
    int id;

    e = apr_thread_mutex_lock(cb.mutex);
    if (e) {
        fprintf(stderr, "fatal: apr_thread_mutex_lock failed\n");
        _exit(1);
    }

    id = ++s_id;

    for (;;) {

        while (cb.ccnt < cb.max && cb.token == 0) {
            printf("consumer %d: wait not_empty\n", id);
            e = apr_thread_cond_wait(cb.not_empty, cb.mutex);
            if (e) {
                fprintf(stderr, "fatal: apr_thread_cond_wait failed\n");
                _exit(1);
            }
            printf("consumer %d: wake\n", id);
        }

        if (cb.ccnt >= cb.max) 
            break;

        printf("consumer %d: consumed %d, signal not_full\n", id, cb.token);
        cb.token = 0;
        ++cb.ccnt;

        e = apr_thread_cond_signal(cb.not_full);
        if (e) {
            fprintf(stderr, "fatal: apr_thread_cond_signal failed\n");
            _exit(1);
        }
    }

    e = apr_thread_mutex_unlock(cb.mutex);
    if (e) {
        fprintf(stderr, "fatal: apr_thread_mutex_unlock failed\n");
        _exit(1);
    }

    printf("consumer %d: exit\n", id);
    
    return 0;
}



void* APR_THREAD_FUNC producer(apr_thread_t* tp, void* _data)
{
    int e;
    static int s_id = 0;
    int id;

    e = apr_thread_mutex_lock(cb.mutex);
    if (e) {
        fprintf(stderr, "fatal: apr_thread_mutex_lock failed\n");
        _exit(1);
    }

    id = ++s_id;

    for (;;) {

        while (cb.pcnt < cb.max && cb.token != 0) {
            printf("producer %d: wait not_full\n", id);
            e = apr_thread_cond_wait(cb.not_full, cb.mutex);
            if (e) {
                fprintf(stderr, "fatal: apr_thread_cond_wait failed\n");
                _exit(1);
            }
            printf("producer %d: wake\n", id);
        }

        if (cb.pcnt >= cb.max) 
            break;

        cb.token = ++cb.pcnt;
        printf("producer %d: produced %d, signal not_empty\n", id, cb.token);
        e = apr_thread_cond_signal(cb.not_empty);
        if (e) {
            fprintf(stderr, "fatal: apr_thread_cond_signal failed\n");
            _exit(1);
        }
    }

    e = apr_thread_mutex_unlock(cb.mutex);
    if (e) {
        fprintf(stderr, "fatal: apr_thread_mutex_unlock failed\n");
        _exit(1);
    }

    printf("producer %d: exit\n", id);
    
    return 0;
}


int main(int argc, const char* argv[])
{
    int np, nc;
    int i, e;
    apr_threadattr_t* attr;
    apr_thread_t** tc;
    apr_thread_t** tp;
    apr_pool_t* ap;

    if (argc != 4) {
        fprintf(stderr, "usage: %s num_producers num_consumers max_goods\n",
                argv[0]);
        exit(1);
    }

    e = apr_initialize();
    if (e) {
        fprintf(stderr, "apr_initialize failed\n");
        exit(1);
    }

    e = apr_pool_create(&ap, 0);
    if (e) {
        fprintf(stderr, "apr_pool_create failed\n");
        exit(1);
    }

    np = atoi(argv[1]);
    nc = atoi(argv[2]);
    cb.max = atoi(argv[3]);

    e = apr_thread_mutex_create(&cb.mutex, APR_THREAD_MUTEX_DEFAULT, ap);
    if (e) {
        fprintf(stderr, "apr_thread_mutex_create failed\n");
        exit(1);
    }

    e = apr_thread_cond_create(&cb.not_full, ap);
    if (e) {
        fprintf(stderr, "apr_thread_cond_create failed\n");
        exit(1);
    }

    e = apr_thread_cond_create(&cb.not_empty, ap);
    if (e) {
        fprintf(stderr, "apr_thread_cond_create failed\n");
        exit(1);
    }

    tp = (apr_thread_t**) apr_pcalloc(ap, sizeof(apr_thread_t*) * np);
    tc = (apr_thread_t**) apr_pcalloc(ap, sizeof(apr_thread_t*) * nc);

    e = apr_threadattr_create(&attr, ap);
    if (e) {
        fprintf(stderr, "apr_threadattr_create failed\n");
        exit(1);
    }

    /*
    e = apr_threadattr_detach_set(attr, 0);
    if (e) {
        char msg[256];
        apr_strerror(e, msg, sizeof(msg));
        fprintf(stderr, "apr_thread_detach_set failed, error %s\n", msg);
        exit(1);
    }
    */
    
    for (i = 0; i < nc; i++) {
        e = apr_thread_create(&tc[i], attr, consumer, 0, ap);
        if (e) {
            fprintf(stderr, "apr_thread_create failed\n");
            exit(1);
        }
    }

    for (i = 0; i < np; i++) {
        e = apr_thread_create(&tp[i], attr, producer, 0, ap);
        if (e) {
            fprintf(stderr, "apr_thread_create failed\n");
            exit(1);
        }
    }

    for (i = 0; i < np; i++) {
        int ret;
        apr_thread_join(&ret, tp[i]);
    }

    apr_thread_cond_broadcast(cb.not_empty);
    for (i = 0; i < nc; i++) {
        int ret;
        apr_thread_join(&ret, tc[i]);
    }

    apr_pool_destroy(ap);
    apr_terminate();

    printf("done\n");
    return 0;
}