4

I have code which is identified by tsan as lock-order-inversion. It is a false positive. I know I can use suppression files to prevent this but I wonder whether I can do something with my code so that my intention is clearer and it doesn't get misidentified. Here's what I'm doing:

void addObject(ObjectData objectData) {
    unique_lock listLock(listMutex);
    ObjectPtr obj = list.createObject(objectData);
    // we need to lock the object here because it's created but not finalized
    // finalization can take time and we don't want to keep the list locked 
    // during that time so that other objects can be added or gotten
    obj->mutex.lock();
    listLock.unlock();

    obj->finalize();
    obj->mutex.unlock();    
}

void cloneObject(ObjectName name) {
    shared_lock listLock(listMutex);
    auto obj = list.getObjectByName(name);
    listLock.unlock();

    // will wait here if object is not finalized
    unique_lock objLock(obj->mutex);
    addObject(obj->data());
}

Here's what tsan observed:

threadA => addObject => list.lock success
threadA => addObject => objX.lock success LIST IS LOCKED
threadA => addObject => list.unlock
// and later:
threadB => cloneObject => objX.lock success
threadB => cloneObject => addObject => list.lock success OBJX IS LOCKED

Here's what it's warning me could happen:

threadA => addObject => list.lock success
threadB => cloneObject => objX.lock success
threadA => addObject => objX.lock waiting
threadB => cloneObject => addObject => list.lock DEAD LOCK

The thing is this can't happen as the only time addObject acquires an object lock is IF the object was just created and there is no way to have a successful lock on an object which could lead to the same object being locked in addObject. Calling addObject from cloneObject will produce an object lock, but this will necessarily be another object and not the one being cloned.

Borislav Stanimirov
  • 1,609
  • 12
  • 23
  • 1
    You should not create the object on the list. Then you don't need to lock the object, and you only need to lock the list while adding the already finished object. – stark Nov 01 '19 at 12:03
  • With what I have, many threads can get the object, start working on other things and only block when they need the finalized object (which is not always what they need... they might only need the metadata, which is already available) – Borislav Stanimirov Nov 01 '19 at 12:14
  • What I hear you saying is that no threads can get the object. They can get a second thing called the object metadata. – stark Nov 01 '19 at 12:20
  • ... which magically turns into an object when you need it to. But yeah. – Borislav Stanimirov Nov 01 '19 at 12:43

0 Answers0