0

I'm trying to implement a threadsafe locking mechanism for an array with the following intended use case:

  1. Request the indexes you want to lock and try to acquire them. If you fail to acquire ANY index, bail out, and try again (essentially spin).
  2. Once the necessary locks have been acquired perform processing on these indexes.
  3. Release the acquired locks!

I'm using the below code to test the lock - it just increments a test count, with the same indexes being specified for each iteration (so it forces access to be sequential). The only problem is it doesn't work, and I'm kind of stumped...

I have a feeling I'm missing some sort of key race condition, but I can't identity it yet :(

#pragma omp parallel for
for (int i = 0; i < activeItems; i++)
{
    std::vector<int> lockedBucketIndexes = {0, 1, 2, 3};
    //try and get a lock on the buckets we want, otherwise keep trying          
    while (!spatialHash->TryAcquireBucketLocks(lockedBucketIndexes))
    {
        //TODO - do some fancy backoff here
    }

    testCount++;

    spatialHash->DropBucketLocks(lockedBucketIndexes);
}

The class/methods that do the locking:

std::vector<int> _bucketLocks;
SpinLock _bucketLockLock;

bool SpatialHash::TryAcquireBucketLocks(std::vector<int> bucketIndexes)
{
    bool success = true;
    //try and get a lock to set our bucket locks... lockception
    _bucketLockLock.lock();

    //quickly check that the buckets we want are free
    for each (int bucketIndex in bucketIndexes)
    {
        if (_bucketLocks[bucketIndex] > 0)
        {
            success = false;
            break;
        }
    }

    //if all the buckets are free, set them to occupied
    if (success)
    {
        for each (int bucketIndex in bucketIndexes)
        {
            _bucketLocks[bucketIndex] = 1;
        }
    }

    //let go of the lock
    _bucketLockLock.unlock();

    return success;
}

void DropBucketLocks(std::vector<int> bucketIndexes)
{
    //I have no idea why these locks are required
    //It seems to almost work with them though...
    _bucketLockLock.lock();

    for each (int bucketIndex in bucketIndexes)
    {
        _bucketLocks[bucketIndex] = 0;
    }

    _bucketLockLock.unlock();

    return true;
}

The spinlock class:

class SpinLock {
    std::atomic_flag locked = ATOMIC_FLAG_INIT;
public:
    void lock() {
        while (locked.test_and_set(std::memory_order_acquire)) { ; }
    }
    void unlock() {
        locked.clear(std::memory_order_release);
    }
}; 
eerorika
  • 232,697
  • 12
  • 197
  • 326
Adam
  • 1
  • @Adam You index `_bucketLocks`, which is an empty `vector`, although it may have been populated somewhere else, not shown here... Can you be more specific on the real problem ? – LWimsey Jan 31 '17 at 02:11
  • What does "it doesn't work" mean? Please provide a [mcve] (read that page carefully!). BTW: [Mixing std::atomic and OpenMP is not portable / standards conforming](http://stackoverflow.com/a/41316118/620382), however I doubt that this is the practical issue you see. – Zulan Jan 31 '17 at 10:30

0 Answers0