C++ std::condition_variable in class context

  c++, multithreading

I am trying to manage prioritized resource access of a camera in an embedded system. Therefore I made a camera interface which uses a std::condition_variable to realize this.

typedef std::function<void()> UnlockCameraHandler;

class Camera {
    const int MAX_PRIORITY = std::numeric_limits<int>::max();
    int currentPriority = MAX_PRIORITY;
    bool mLockable = true;
    UnlockCameraHandler mUnlockCameraHandler;
    std::condition_variable_any cameraLock;
    std::mutex cameraMut;

public:
    bool isLockable(int priority) {
      if (priority < currentPriority) {
        return true;
      } else {
        return false;
      }
    }

    void unlock() {
      if (mLockable) {
        return;
      }
      currentPriority = MAX_PRIORITY;
      mLockable = true;
      cameraLock.notify_all();
    }

    bool lock(int priority, UnlockCameraHandler handler) {
      if (priority < 0) {
        return false;
      }
      if (priority < currentPriority) {
        if (mUnlockCameraHandler) {
          mUnlockCameraHandler();
        }
      }
      if (!mLockable) {
        std::unique_lock<std::mutex> mLock(cameraMut);
        cameraLock.wait(mLock, std::bind(&Camera::isLockable, this, priority));
      }
      std::lock_guard<std::mutex> guard(cameraMut);
      mUnlockCameraHandler = handler;
      currentPriority = priority;
      mLockable = false;
      return true;
    }
};

I wrote some Catch2 Unit-Tests for these functions to also ensure that it works with more than one thread.

struct CameraLock {
  std::shared_ptr<TestCamera> camera;

  void unlock_handler() { camera->unlock(); }
};

TEST_CASE_METHOD(CameraLock, "Camera lock mechanism") {

  SECTION("lock and unlock camera") {
    camera->lock(100, std::bind(&CameraLock::unlock_handler, this));
    REQUIRE(!camera->isLockable(100));
    camera->unlock();
    REQUIRE(camera->isLockable(100));
  }

  SECTION("lock with higher priority") {
    std::thread low{[&]() {
      REQUIRE(camera->lock(100, std::bind(&CameraLock::unlock_handler, this)));
    }};
    std::thread high{[&]() {
      REQUIRE(camera->lock(50, std::bind(&CameraLock::unlock_handler, this)));
    }};
    low.join();
    high.join();
  }

  SECTION("lock with lower priority") {
    std::thread high{[&]() {
      REQUIRE(camera->lock(50, std::bind(&CameraLock::unlock_handler, this)));
      std::this_thread::sleep_for(std::chrono::milliseconds(10));
      camera->unlock();
    }};

    std::thread low{[&]() {
      REQUIRE(camera->lock(100, std::bind(&CameraLock::unlock_handler, this)));
      camera->unlock();
    }};
    high.join();
    low.join();
    REQUIRE(camera->isLockable(100));
  }

  SECTION("lock with 3 parties") {
    std::thread high{[&]() {
      REQUIRE(camera->lock(50, std::bind(&CameraLock::unlock_handler, this)));
      std::this_thread::sleep_for(std::chrono::milliseconds(10));
      camera->unlock();
    }};

    std::thread low{[&]() {
      REQUIRE(camera->lock(100, std::bind(&CameraLock::unlock_handler, this)));
      camera->unlock();
    }};

    std::thread mid{[&]() {
      REQUIRE(camera->lock(75, std::bind(&CameraLock::unlock_handler, this)));
      std::this_thread::sleep_for(std::chrono::milliseconds(5));
      camera->unlock();
    }};
    high.join();
    low.join();
    mid.join();
    REQUIRE(camera->isLockable(100));
  }
}

It should work in that way that when a process wants to access the camera resource, it calls lock() to reserve it, passing a priority value and a Callback function. The Callback function serves the purpose that if another process wants to access the camera with a higher priority, the process that currently has the access does the appropriate work to free the resource and calls unlock() afterwards, so that the prioritized process can lock it.
If a process with a lower priority wants to access the resource, it calls the wait() function of the condition_variable and blocks until the prioritized process calls unlock().

The Unit-Tests do pass most of the time, but sometimes I’m getting a segmentation fault or a double free() error (especially in the "lock with 3 parties" section) which tells me that my implementation isn’t thread safe.

May someone tell me what’s wrong with my code and how to adjust it to meet my exceptations?
Thanks in advance!

Source: Windows Questions C++

LEAVE A COMMENT