1

Trying to understand what is going on here and why some of the destructors are not getting called. When I run the program (example below), I expected an output coming from the destructors of all SPI/I2C and GPIO peripheral classes, but they did not get called. Instead I only see the output called from the destructor of the SystemProvider class.

Note: still a beginner on C++

Header:

#include <string>
#include <memory>

class Provider;

// =========================================================================================
//                           TOP LEVEL PERIPHERAL CLASS
// =========================================================================================

class Peripheral {
public:

    explicit Peripheral(std::shared_ptr<Provider> provider);

    virtual ~Peripheral();

    [[nodiscard]] const std::shared_ptr<Provider> &getProvider() const;

private:
    std::shared_ptr<Provider> m_Provider;
};

// =========================================================================================
//                           BASE PERIPHERAL CLASSES
// =========================================================================================

class I2CPeripheral : public Peripheral {
public:
    explicit I2CPeripheral(std::shared_ptr<Provider> provider);

    ~I2CPeripheral() override;

    virtual void write(int address, char* buf, int size) = 0;
};

class GpioPeripheral : public Peripheral {
public:
    explicit GpioPeripheral(std::shared_ptr<Provider> provider);

    ~GpioPeripheral() override;

    virtual void init(int pin, int mode) = 0;

    virtual void write(int pin, int value) = 0;
};

class SpiPeripheral : public Peripheral {
public:
    explicit SpiPeripheral(std::shared_ptr<Provider> provider);

    ~SpiPeripheral() override;

    virtual void write(char *buffer, int size) = 0;
};

// =========================================================================================
//               CONCRETE (PROVIDER SPECIFIC) PERIPHERAL CLASSES
// =========================================================================================

class SystemI2CPeripheral : public I2CPeripheral {
public:
    explicit SystemI2CPeripheral(std::shared_ptr<Provider> provider);

    ~SystemI2CPeripheral() override;

    void write(int address, char *buf, int size) override;
};

class SystemGpioPeripheral : public GpioPeripheral {
public:
    explicit SystemGpioPeripheral(std::shared_ptr<Provider> provider);

    ~SystemGpioPeripheral() override;

    void init(int pin, int mode) override;

    void write(int pin, int value) override;
};

class SystemSpiPeripheral : public SpiPeripheral {
public:
    explicit SystemSpiPeripheral(std::shared_ptr<Provider> provider);

    ~SystemSpiPeripheral() override;

    void write(char *buffer, int size) override;
};

class PigpioGpioPeripheral : public GpioPeripheral {
public:
    explicit PigpioGpioPeripheral(std::shared_ptr<Provider> provider);

    ~PigpioGpioPeripheral() override;

    void init(int pin, int mode) override;

    void write(int pin, int value) override;
};

class PigpioI2CPeripheral : public I2CPeripheral {
public:
    explicit PigpioI2CPeripheral(std::shared_ptr<Provider> provider);

    ~PigpioI2CPeripheral() override;

    void write(int address, char *buf, int size) override;
};

class PigpioSpiPeripheral : public SpiPeripheral {
public:
    explicit PigpioSpiPeripheral(std::shared_ptr<Provider> provider);

    ~PigpioSpiPeripheral() override;

    void write(char *buffer, int size) override;
};

// =========================================================================================
//                           BASE PROVIDER CLASS
// =========================================================================================

class Provider {
public:
    Provider() = default;

    explicit Provider(std::string name);

    virtual ~Provider();

    std::string getName();

    virtual std::shared_ptr<GpioPeripheral> getGpioPeripheral();

    virtual std::shared_ptr<I2CPeripheral> getI2CPeripheral();

    virtual std::shared_ptr<SpiPeripheral> getSpiPeripheral();
protected:
    std::shared_ptr<GpioPeripheral> gpioPeripheral;

    std::shared_ptr<SpiPeripheral> spiPeripheral;

    std::shared_ptr<I2CPeripheral> i2cPeripheral;

    virtual std::shared_ptr<GpioPeripheral> createGpioPeripheral() = 0;

    virtual std::shared_ptr<I2CPeripheral> createI2CPeripheral() = 0;

    virtual std::shared_ptr<SpiPeripheral> createSpiPeripheral() = 0;

private:
    std::string name;
};

// =========================================================================================
//                           CONCRETE PROVIDER CLASSES
// =========================================================================================

class SystemProvider : public Provider, public std::enable_shared_from_this<SystemProvider> {
public:
    SystemProvider();

    ~SystemProvider() override;

    std::shared_ptr<SystemProvider> getPointer() {
        return this->shared_from_this();
    }
protected:
    std::shared_ptr<GpioPeripheral> createGpioPeripheral() override;

    std::shared_ptr<I2CPeripheral> createI2CPeripheral() override;

    std::shared_ptr<SpiPeripheral> createSpiPeripheral() override;
};

class PigpioProvider : public Provider, public std::enable_shared_from_this<PigpioProvider> {
public:

    PigpioProvider();

    ~PigpioProvider() override;

    std::shared_ptr<PigpioProvider> getPointer() {
        return this->shared_from_this();
    }
protected:
    std::shared_ptr<GpioPeripheral> createGpioPeripheral() override;

    std::shared_ptr<I2CPeripheral> createI2CPeripheral() override;

    std::shared_ptr<SpiPeripheral> createSpiPeripheral() override;
};

Source:

#include "Peripherals.h"

#include "Common.h"

Peripheral::Peripheral(std::shared_ptr<Provider> provider) : m_Provider(provider) {
    debug("Peripheral: constructor");
}

Peripheral::~Peripheral() {
    debug("Peripheral: destructor");
}

const std::shared_ptr<Provider> &Peripheral::getProvider() const {
    return m_Provider;
}

I2CPeripheral::I2CPeripheral(std::shared_ptr<Provider> provider) : Peripheral(provider) {
    debug("\tI2CPeripheral: constructor");
}

I2CPeripheral::~I2CPeripheral() {
    debug("I2CPeripheral: destructor");
}

GpioPeripheral::GpioPeripheral(std::shared_ptr<Provider> provider) : Peripheral(provider) {
    debug("\tGpioPeripheral: constructor");
}

GpioPeripheral::~GpioPeripheral() {
    debug("GpioPeripheral: destructor");
}

SpiPeripheral::SpiPeripheral(std::shared_ptr<Provider> provider) : Peripheral(provider) {
    debug("\tSpiPeripheral: constructor");
}

SpiPeripheral::~SpiPeripheral() {
    debug("SpiPeripheral: destructor");
}

SystemI2CPeripheral::SystemI2CPeripheral(std::shared_ptr<Provider> provider) : I2CPeripheral(provider) {
    debug("\t\tSystemI2CPeripheral: constructor");
}

SystemI2CPeripheral::~SystemI2CPeripheral() {
    debug("SystemI2CPeripheral: destructor");
}

void SystemI2CPeripheral::write(int address, char *buf, int size) {

}

SystemGpioPeripheral::SystemGpioPeripheral(std::shared_ptr<Provider> provider) : GpioPeripheral(provider) {
    debug("\t\tSystemGpioPeripheral: constructor");
}

SystemGpioPeripheral::~SystemGpioPeripheral() {
    debug("SystemGpioPeripheral: destructor");
}

void SystemGpioPeripheral::init(int pin, int mode) {

}

void SystemGpioPeripheral::write(int pin, int value) {

}

SystemSpiPeripheral::SystemSpiPeripheral(std::shared_ptr<Provider> provider) : SpiPeripheral(provider) {
    debug("\t\tSystemSpiPeripheral: constructor");
}

SystemSpiPeripheral::~SystemSpiPeripheral() {
    debug("SystemSpiPeripheral: destructorr");
}

void SystemSpiPeripheral::write(char *buffer, int size) {
    debug(std::string(__func__) + std::string(": Writing to SPI peripheral with ") + std::to_string(size) + std::string(" bytes"));
}

PigpioGpioPeripheral::PigpioGpioPeripheral(std::shared_ptr<Provider> provider) : GpioPeripheral(provider) {
    debug("\t\tPigpioGpioPeripheral: constructor");
}

PigpioGpioPeripheral::~PigpioGpioPeripheral() {
    debug("PigpioGpioPeripheral: destructor");
}

void PigpioGpioPeripheral::init(int pin, int mode) {

}

void PigpioGpioPeripheral::write(int pin, int value) {

}

PigpioI2CPeripheral::PigpioI2CPeripheral(std::shared_ptr<Provider> provider) : I2CPeripheral(provider) {
    debug("\t\tPigpioI2CPeripheral: constructor");
}

PigpioI2CPeripheral::~PigpioI2CPeripheral() {
    debug("PigpioI2CPeripheral: destructor");
}

void PigpioI2CPeripheral::write(int address, char *buf, int size) {

}

PigpioSpiPeripheral::PigpioSpiPeripheral(std::shared_ptr<Provider> provider) : SpiPeripheral(provider) {
    debug("\t\tPigpioSpiPeripheral: constructor");
}

PigpioSpiPeripheral::~PigpioSpiPeripheral() {
    debug("PigpioSpiPeripheral: destructor");
}

void PigpioSpiPeripheral::write(char *buffer, int size) {
    debug(std::string(__func__) + std::string(": Writing to SPI peripheral with ") + std::to_string(size) + std::string(" bytes"));
}

Provider::Provider(std::string name) : name(std::move(name)) {
    debug("Provider: constuctor");
}

Provider::~Provider() {
    debug("\tProvider: destructor");
    gpioPeripheral = nullptr;
    spiPeripheral = nullptr;
    i2cPeripheral = nullptr;
}

std::string Provider::getName() {
    return name;
}

std::shared_ptr<GpioPeripheral> Provider::getGpioPeripheral() {
    if (gpioPeripheral == nullptr) {
        debug(std::string("Creating a new instance of gpio peripheral for provider: ") + getName());
        gpioPeripheral = createGpioPeripheral();
    } else {
        debug("Using existing instance of gpioPeripheral");
    }
    return gpioPeripheral;
}

std::shared_ptr<I2CPeripheral> Provider::getI2CPeripheral() {
    if (i2cPeripheral == nullptr) {
        debug(std::string("Creating a new instance of i2c peripheral for provider: ") + getName());
        i2cPeripheral = createI2CPeripheral();
    }
    return i2cPeripheral;
}

std::shared_ptr<SpiPeripheral> Provider::getSpiPeripheral() {
    if (spiPeripheral == nullptr) {
        debug(std::string("Creating a new instance of spi peripheral for provider: ") + getName());
        spiPeripheral = createSpiPeripheral();
    }
    return spiPeripheral;
}

SystemProvider::SystemProvider() : Provider("system") {
    debug("\tSystemProvider: constructor");
}

SystemProvider::~SystemProvider() {
    debug("SystemProvider: destructor");
}

std::shared_ptr<GpioPeripheral> SystemProvider::createGpioPeripheral() {
    return std::make_shared<SystemGpioPeripheral>(getPointer());
}

std::shared_ptr<I2CPeripheral> SystemProvider::createI2CPeripheral() {
    return std::make_shared<SystemI2CPeripheral>(getPointer());
}

std::shared_ptr<SpiPeripheral> SystemProvider::createSpiPeripheral() {
    return std::make_shared<SystemSpiPeripheral>(getPointer());
}

PigpioProvider::PigpioProvider() : Provider("pigpio") {
    debug("\tPigpioProvider: constructor");
}

PigpioProvider::~PigpioProvider() {
    debug("PigpioProvider: destructor");
}

std::shared_ptr<GpioPeripheral> PigpioProvider::createGpioPeripheral() {
    return std::make_shared<PigpioGpioPeripheral>(getPointer());
}

std::shared_ptr<I2CPeripheral> PigpioProvider::createI2CPeripheral() {
    return std::make_shared<PigpioI2CPeripheral>(getPointer());
}

std::shared_ptr<SpiPeripheral> PigpioProvider::createSpiPeripheral() {
    return std::make_shared<PigpioSpiPeripheral>(getPointer());
}

Main Executable:

void debug(const std::string& msg) {
    std::cout << "[debug] " << msg << std::endl;
}

int main() {
    //Two providers available for SPI/I2C and GPIO peripherals
    debug("==============================================================");
    debug("Peripheral Providers (Instantiate both)");
    debug("==============================================================");
    std::shared_ptr<PigpioProvider> pigpioProvider = std::make_shared<PigpioProvider>();
    std::shared_ptr<SystemProvider> systemProvider = std::make_shared<SystemProvider>();

    debug("==============================================================");
    debug("Retrieve peripherals from the selected provider");
    debug("==============================================================");

    //Choose a default peripheral provider (pigpioProvider or systemProvider)
    std::shared_ptr<Provider> defaultProvider = pigpioProvider;
    //std::shared_ptr<Provider> defaultProvider = systemProvider;

    //Retrieve the peripheral interfaces
    std::shared_ptr<GpioPeripheral> gpio = defaultProvider->getGpioPeripheral();
    std::shared_ptr<I2CPeripheral> i2c = defaultProvider->getI2CPeripheral();
    std::shared_ptr<SpiPeripheral> spi = defaultProvider->getSpiPeripheral();

    //Test
    gpio->write(10, 1);
    i2c->write(0x10, nullptr, 0);
    spi->write(nullptr, 0);

    debug("==============================================================");
    debug("Exiting program...");
    debug("==============================================================");

    return 0;
}

Output:

[debug] ==============================================================
[debug] Peripheral Providers (Instantiate both)
[debug] ==============================================================
[debug] Provider: constuctor
[debug]     PigpioProvider: constructor
[debug] Provider: constuctor
[debug]     SystemProvider: constructor
[debug] ==============================================================
[debug] Retrieve peripherals from the selected provider
[debug] ==============================================================
[debug] Creating a new instance of gpio peripheral for provider: pigpio
[debug] Peripheral: constructor
[debug]     GpioPeripheral: constructor
[debug]         PigpioGpioPeripheral: constructor
[debug] Creating a new instance of i2c peripheral for provider: pigpio
[debug] Peripheral: constructor
[debug]     I2CPeripheral: constructor
[debug]         PigpioI2CPeripheral: constructor
[debug] Creating a new instance of spi peripheral for provider: pigpio
[debug] Peripheral: constructor
[debug]     SpiPeripheral: constructor
[debug]         PigpioSpiPeripheral: constructor
[debug] write: Writing to SPI peripheral with 0 bytes
[debug] ==============================================================
[debug] Exiting program...
[debug] ==============================================================
[debug] SystemProvider: destructor
[debug]     Provider: destructor

Take note of the output at the end:

[debug] SystemProvider: destructor
[debug]     Provider: destructor

But I expected an output that would look somewhat like this:

[debug] PigpioProvider: destructor
[debug]     Provider: destructor
[debug] PigpioGpioPeripheral: destructor
[debug]     GpioPeripheral: destructor
[debug]         Peripheral: destructor
[debug] PigpioI2CPeripheral: destructor
[debug]     I2cPeripheral: destructor
[debug]         Peripheral: destructor
[debug] PigpioSpiPeripheral: destructor
[debug]     SpiPeripheral: destructor
[debug]         Peripheral: destructor
Rafael Ibasco
  • 1,342
  • 3
  • 14
  • 19
  • Please take the time to read about and appreciate the concept of a [mre]. The class diagram and your entire code are hardly needed here to demonstrate your problem. – StoryTeller - Unslander Monica Nov 17 '19 at 07:30
  • Please try to post only the minimum code needed to reproduce your problem. Otherwise, it's a lot of code for someone who's never seen it before to decipher. –  Nov 17 '19 at 07:33
  • I haven't read your whole code but you probably have shared pointer cycles, you should replace some with weak_ptr to break the cycles. – Alan Birtles Nov 17 '19 at 07:44
  • @Chipster I'll try to minimize it, I just wasn't sure where exactly the problem is coming from and it seems to occur when all of these components have been combined all together. – Rafael Ibasco Nov 17 '19 at 07:54
  • You have `std::shared_ptr` in your `Peripheral` objects, and `std::shared_ptr` in your `Provider` objects, where `XXXX` are `Peripheral` derivations. The latter points to the former, which points to the latter. That's a cycle. It at-least-looks like the `Peripheral` objects should host a `std::weak_ptr` instead of a strong `std::shared_ptr`. More work to get the actual shared pointer when you need it (*if* you need it) in peripherals, but it should solve your issue. – WhozCraig Nov 17 '19 at 07:56

2 Answers2

2

Is the provider who owns the peripheral or are the peripherals who own the provider?

You used std::shared_ptr for both the directions, thus creating ownership cycles.

You have to decide, in your design, which relation is forward, (to be owned by shared_ptr) and which is backward (to referred by weak_ptr, to assign to shared_ptr only temporarily)

Emilio Garavaglia
  • 20,229
  • 2
  • 46
  • 63
2

Your Provider class maintains shared_ptrs of peripherals. For example:

class Provider
{
public:
  Provider() = default;
  // ...
protected:
  std::shared_ptr<GpioPeripheral> gpioPeripheral;
  std::shared_ptr<SpiPeripheral> spiPeripheral;
  std::shared_ptr<I2CPeripheral> i2cPeripheral;
};

But, your peripherals also maintain a shared_ptr of the default provider, due to inheriting from the Peripheral parent class:

class Peripheral
{
public:
  explicit Peripheral(std::shared_ptr<Provider> provider);
  // ...
private:
  std::shared_ptr<Provider> m_Provider;
};

This results in a circular dependency chain, causing the shared_ptr to never get cleaned up.

I'd suggest changing your m_Provider to std::weak_ptr instead of std::shared_ptr. I don't believe from the example you've shared that the peripherals own the provider.

Samuel O'Malley
  • 3,471
  • 1
  • 23
  • 41