[cfe-commits] patch: libcxxabi cxa_*_virtual and cxa_guard_* methods

[Howard Hinnant]

On May 20, 2011, at 2:11 AM, Nick Lewycky wrote:

> I've started dipping my toes in the libcxxabi project by implementing a few simple methods. Patch attached, please review!
> 
> This isn't heavily tested, I'm mostly just trying to make sure I have the style down. Please let me know if we should be using different file names (hard to follow a pattern where there isn't one), of if the private methods in cxa_guard actually belong in an anonymous namespace, etc.
> 
> Nick
> 
> <libcxxabi-virtual-and-guard.patch>

I disagree with John that your implementation doesn't support the recursive calls for *different* static objects.  I believe it does.  And I like that your design allows for parallel initialization of different static objects.

What I'm nervous about though is the use of the spin lock.  An arbitrary amount of time can go by while one thread is initializing and the other thread is spinning, waiting for that initialization to complete.

I translated your code into using pthread_mutex_t and pthread_cond_t, and came up with the following.  I tried to keep your notation so as to make the two implementations more easily comparable:

#include <pthread.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

namespace __cxxabiv1
{

namespace __libcxxabi
{

void abort(const char* s) {printf("%s\n", s); ::abort();}

}  // __libcxxabi

static pthread_mutex_t guard_mut = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t  guard_cv  = PTHREAD_COND_INITIALIZER;

extern "C"
{

int __cxa_guard_acquire(int64_t* guard_object)
{
    uint8_t* initialized = (uint8_t*)guard_object;
    uint8_t* lock = (uint8_t*)guard_object + 1;
    int r = pthread_mutex_lock(&guard_mut);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_acquire failed to acquire mutex");
    if (*initialized == 1)
    {
        r = pthread_mutex_unlock(&guard_mut);
        if (r != 0)
            __libcxxabi::abort("__cxa_guard_acquire failed to release mutex");
        return 0;
    }
    while (*lock == 1)
        pthread_cond_wait(&guard_cv, &guard_mut);
    int result = *initialized;
    if (result == 0)
        *lock = 1;
    r = pthread_mutex_unlock(&guard_mut);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_acquire failed to release mutex");
    return result == 0;
}

void __cxa_guard_release(int64_t* guard_object)
{
    uint8_t* initialized = (uint8_t*)guard_object;
    uint8_t* lock = (uint8_t*)guard_object + 1;
    int r = pthread_mutex_lock(&guard_mut);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_release failed to acquire mutex");
    *initialized = 1;
    *lock = 0;
    r = pthread_mutex_unlock(&guard_mut);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_release failed to release mutex");
    r = pthread_cond_broadcast(&guard_cv);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_release failed to broadcast condition variable");
}

void __cxa_guard_abort(int64_t* guard_object)
{
    uint8_t* lock = (uint8_t*)guard_object + 1;
    int r = pthread_mutex_lock(&guard_mut);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_abort failed to acquire mutex");
    *lock = 0;
    r = pthread_mutex_unlock(&guard_mut);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_abort failed to release mutex");
    r = pthread_cond_broadcast(&guard_cv);
    if (r != 0)
        __libcxxabi::abort("__cxa_guard_abort failed to broadcast condition variable");
}

}  // extern "C"

}  // __cxxabiv1

The structure of our implementations is nearly identical.

One of the things I don't like about my implementation (or yours) is that it does not detect recursive entry of the same static object.  I'm thinking of something that makes use of thread_local data get that part, but I'm not there yet.  But I thought I'd share what I have so far.

Thanks for helping us with this library!

Howard