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 agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.