1

I have the following code:

class Cohomology;

struct EMField
{
     std::shared_ptr<Cohomology> coh;
     std::array<DIM> data;

     // other methods

}

class Cohomology
{
     private:
        // private members
     public:
        Cohomology(PList params)
        {
             // Constructor of the class
        }
        
        virtual ~Cohomology() {std::cout << "Cohomology destroyed" << std::endl;}

        void initializeField(EMField& field)
        {
             field.coh.reset(this);
             // other methods to initialize field.data using the private members
        }
}

But the class Cohomology also has virtual methods that are implemented by SubCohomology:

class SubCohomology : public Cohomology
{
     public:
        SubCohomology(PList params) {}
        
        ~Cohomology() {std::cout << "SubCohomology destroyed" << std::endl;}

        // Implementation of the virtual methods
}

So a test code to check whether EMFields are initialized and can be manipulated looks like:

int main(int argc, char *argv[])
{
     // variables needed to initialize PList params
     PList params(); // construct params

     SubCohomology coh(params);

     EMField field;

     coh.initializeField(field);

}

The code compiles, but running it yields this error:

SubCohomology destroyed
Cohomology destroyed
free(): invalid pointer
[machine:324808] *** Process received signal ***
[machine:324808] Signal: Aborted (6)
[machine:324808] Associated errno: Unknown error 32767 (32767)
[machine:324808] Signal code:  (24)
[machine:324808] [ 0] /usr/lib/libc.so.6(+0x38a40)[0x7f4ac0054a40]
[machine:324808] [ 1] /usr/lib/libc.so.6(+0x884dc)[0x7f4ac00a44dc]
[machine:324808] [ 2] /usr/lib/libc.so.6(gsignal+0x18)[0x7f4ac0054998]
[machine:324808] [ 3] /usr/lib/libc.so.6(abort+0xd7)[0x7f4ac003e53d]
[machine:324808] [ 4] /usr/lib/libc.so.6(+0x7c67e)[0x7f4ac009867e]
[machine:324808] [ 5] /usr/lib/libc.so.6(+0x9226c)[0x7f4ac00ae26c]
[machine:324808] [ 6] /usr/lib/libc.so.6(+0x940bc)[0x7f4ac00b00bc]
[machine:324808] [ 7] /usr/lib/libc.so.6(__libc_free+0x73)[0x7f4ac00b2a33]
[machine:324808] [ 8] /home/user/builddir/test_fields(_ZN13EMFieldILi0ELi1EED2Ev+0x83)[0x556db1fc0f73]
[machine:324808] [ 9] /home/user/builddir/test_fields(main+0x36e)[0x556db1fa205e]
[machine:324808] [10] /usr/lib/libc.so.6(+0x232d0)[0x7f4ac003f2d0]
[machine:324808] [11] /usr/lib/libc.so.6(__libc_start_main+0x8a)[0x7f4ac003f38a]
[machine:324808] [12] /home/user/builddir/test_fields(_start+0x25)[0x556db1fa3ba5]
[machine:324808] *** End of error message ***
Aborted (core dumped)

which happens after the function initializeField. It is a memory problem, which might be related to trying to free() a non-existing resource.

I suspect that using std::enable_shared_from_this might be helpful to address this problem but I don't know how to implement the mandatory inheritance considering my particular problem, as I am trying to initialize the std::shared_ptr<Cohomology> coh class member of a field in the Cohomology class.

The example outlined here is very helpful to understand how to use this, but I don't know if I would have to nest another struct in EMField to implement this. I also understand the problem solved in this question: when should we use std::enable_shared_from_this, but I cannot put it in the context where a struct has a std::shared_ptr as a member.

Please understand that many EMField objects might be added, whose std::shared_ptr<Cohomology> member points for all fields to the same object

Thank you.

rungekutta
  • 39
  • 5
  • 3
    `coh` is not dynamically-allocated. A pointer to it should never be stored in a `std::shared_ptr`. – Miles Budnek Aug 17 '22 at 15:02
  • I cannot declare ```Cohomology``` as a member because I would otherwise get an incomplete type error. That is why I declared it as a pointer, as explained here https://stackoverflow.com/a/553869/15547591. Moreover, many ```EMField``` objects' members will need to have this member as being a pointer to the same ```coh``` object. – rungekutta Aug 17 '22 at 15:11

2 Answers2

3

std::shared_ptr exists to manage the lifetime of dynamically-allocated objects. No such management is needed (or possible) for an object with automatic storage duration (like coh). Its lifetime is tied to its enclosing scope. Therefore a pointer to coh must never be managed by a std::shared_ptr.

Instead, you should consider creating a constructor in EMField that accepts a std::shared_ptr<Cohomology> and having the caller create an appropriate dynamically-allocated object:

struct EMField
{
    std::shared_ptr<Cohomology> coh;
    // ...

    EMField(std::shared_ptr<Cohomology> c)
        : coh{c}
    {}

    // ...
};

int main(int argc, char *argv[])
{
    // variables needed to initialize PList params
    PList params(); // construct params

    auto coh = std::make_shared<SubCohomology>(params);

    EMField field(coh);

    // No longer needed since EMField's constructor initializes its
    // fields now
    // coh.initializeField(field);
}

Demo


If you absolutely don't want to have to pass your Cohomology object into EMField from the caller, and all Cohomology objects should be dynamically-allocated, and they should all be managed by std::shared_ptrs, then, and only then, is std::enable_shared_from_this the tool for the job.

Example:

class Cohomology : public std::enable_shared_from_this<Cohomology>
{
    private:
        // private members
    protected:
        Cohomology(PList params)
        {
             // Constructor of the class
        }

        Cohomology(const Cohomology&) = delete;

    public:
        virtual ~Cohomology()
        {
            std::cout << "Cohomology destroyed\n";
        }

        static std::shared_ptr<Cohomology> create(PList params)
        {
            return std::shared_ptr<Cohomology>(new Cohomology(params));
        }

        void initializeField(EMField& field)
        {
             field.coh = shared_from_this();
             // ...
        }

        // ...
};

class SubCohomology : public Cohomology
{
    private:
        SubCohomology(PList params)
            : Cohomology(params)
        {}

    public:
        ~SubCohomology()
        {
            std::cout << "SubCohomology destroyed\n";
        }
        
        static std::shared_ptr<SubCohomology> create(PList params)
        {
            return std::shared_ptr<SubCohomology>(new SubCohomology(params));
        }

        // Implementation of the virtual methods
};

int main(int argc, char *argv[])
{
    // variables needed to initialize PList params
    PList params; // construct params

    std::shared_ptr<SubCohomology> coh = SubCohomology::create(params);

    EMField field;

    coh->initializeField(field);
}

Demo

Note that Cohomology and SubCohomology now have non-public constructors. If you inherit from std::enable_shared_from_this you should not allow any objects to ever not be managed by a std::shared_ptr, so separate factory functions are needed to ensure that fact.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
0
  • initializeField reset your smart pointer : delete the object
  • i think that this method is not in the right object, you may do it outside the Cohomology object
  • I cannot use the ```operator=``` unless I use a raw pointer, which is something I am trying to avoid. This method needs to be in the ```Cohomology``` class as I would like the field's ```coh``` member to be initialized with a ```this``` pointer, being ```this``` a pointer to ```SubCohomology```. Do you know of a better way to work around this ? – rungekutta Aug 17 '22 at 15:17
  • do you tried to add: field.coh = std::make_shared(); in your main just after instanciate field ? – casaDePapel Aug 17 '22 at 15:24
  • Yes, but this is not possible because Cohomology is an abstract class, it would need to be added in the SubCohomology one, which implements the virtual methods of ```Cohomology```. But then I cannot access the private members of ```Cohomology``` to initialize the field.data members – rungekutta Aug 17 '22 at 15:27
  • i think that the main problem is your design, if you can avoid cyclic dependency is will be better. anyway, you can convert the initializeField method to virtual and in the derived class you can do comething like : void initializeField(EMField& field) { Cohomology::initializeField(field); field.coh.reset(this); // other methods to initialize field.data using the private members } – casaDePapel Aug 17 '22 at 15:41