2
\$\begingroup\$

I have a thread-safe API for an observable Sensor. The concrete type of the Sensor is chosen in the configure method (strategy pattern). To avoid deadlock when calling the stopmethod, I had to use two mutexes. The first one is to protect the observers list obs_ and the other one is to protect the strategy pointer sensor_.

I am not a fan of using multiple mutexes to make a thread-safe API. My hunch is that there may be a better way to do that.

#include <memory>
#include <thread>
#include <mutex>
#include <list>
#include <iostream>

class SensorImpl
{
public:
    virtual void start() = 0;
    virtual void stop() = 0;
};

class SensorA : public SensorImpl
{
public:
    typedef std::function<void(int)> DataCallback;

    SensorA(DataCallback cb) :
        dataCallback_(cb)
    { }

    virtual ~SensorA() { }

    void start() override
    {
        running_ = true;
        workerThread_.reset(new std::thread(&SensorA::dataProducer, this));
    }

    void stop() override
    {
        if(workerThread_)
        {
            running_ = false;
            workerThread_->join();
            workerThread_.reset();
        }
    }

private:

    void dataProducer()
    {
        while(running_)
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(100));
            int data = 6;
            dataCallback_(data);
        }
    }

    DataCallback dataCallback_;
    std::unique_ptr<std::thread> workerThread_;
    bool running_{false};
};

class SensorObserver
{
    virtual void configured() = 0;
    virtual void started() = 0;
    virtual void dataReceived(int data) = 0;
    virtual void stopped() = 0;
    virtual void cleanup() = 0;
};

// Sensor API must be thread-safe
class Sensor
{
public:

    Sensor() { }
    ~Sensor() { }

    void configure()
    {
        std::lock_guard<std::mutex> lock(implMtx_);

        auto dataCallback =
            [this](int data)
            {
                std::lock_guard<std::mutex> lock(obsMtx_);
                std::cout << "dataReceived" << std::endl;
                // notify SensorObserver::dataReceived
            };

        sensor_.reset(new SensorA(dataCallback));
        {
            std::lock_guard<std::mutex> lock(obsMtx_);
            std::cout << "configured" << std::endl;
            // notify SensorObserver::configured()
        }
    }

    void start()
    {
        std::lock_guard<std::mutex> lock(implMtx_);
        if(sensor_)
        {
            sensor_->start(); 
            {
                std::lock_guard<std::mutex> lock(obsMtx_);
                std::cout << "started" << std::endl;
                // notify SensorObserver::started()
            }
        }
    }

    void stop()
    {
        std::lock_guard<std::mutex> lock(implMtx_);
        if(sensor_)
        {
            sensor_->stop();
            {
                std::lock_guard<std::mutex> lock(obsMtx_);
                std::cout << "stopped" << std::endl;
                // notify SensorObserver::stopped()
            }
        }
    }

    void cleanup()
    {
        std::lock_guard<std::mutex> lock(implMtx_);
        sensor_.reset();
        {
            std::lock_guard<std::mutex> lock(obsMtx_);
            std::cout << "cleanup" << std::endl;
            // notify SensorObserver::cleanup()
        }
    }

    // Observer attach/detach/notify code omitted

private:

    std::mutex obsMtx_, implMtx_;
    std::unique_ptr<SensorImpl> sensor_;
    std::list<SensorObserver*> obs_;
};

int main()
{
    Sensor sensor;

    sensor.configure();
    sensor.start();

    std::this_thread::sleep_for(std::chrono::milliseconds(500));

    sensor.stop();
    sensor.cleanup();

    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ You appear to have tested this code, but it looks like it doesn't work as intended. Please clarify after taking a look at the help center. \$\endgroup\$ – Mast Jan 22 at 15:07
  • \$\begingroup\$ @TobySpeight @Mast Thank you for educating me as a new contributor. I edited the question with the working code. \$\endgroup\$ – Philippe Jan 22 at 15:25

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Browse other questions tagged or ask your own question.