areuz areuz - 2 months ago 6
C++ Question

Seg. Fault on exit caused by pointer to vector of pointers to struct

After a few days of delaying this question and debugging I've found that If my code below is run after removing the vector from the union (and all of the code mentioning this vector) the seg. fault dissapears.

#include <string>
#include <vector>
#include <functional>

struct Task
{
std::string Name;

std::vector<Task*> *Subtasks;
std::function<void(std::string const &)> Job;

Task() {}
Task(std::string const &arg_0) { Name = arg_0; }
Task(std::string const &arg_0, std::vector<Task*> *arg_1) { Name = arg_0; Subtasks = arg_1; }
Task(std::string const &arg_0, std::function<void(std::string const &)> arg_1)
{ Name = arg_0; Job = arg_1; }

~Task() { for (auto tItem : *Subtasks) { delete tItem; } }
};

class Console
{
private:
std::vector<Task*> Routine;

public:
~Console() { for (auto tItem : Routine) { delete tItem; } } //I thought that this is not needed but Valgrind thinks otherwise, strangely the deconstructors of the Tasks are not called, I guess.

void add(Task *arg_0) { Routine.push_back(arg_0); }

void foo()
{
Task *tTask = new Task();
//Task *tTask = new Task("Name");
//Task *tTask = new Task("Name", [this](std::string const &arg_0){ ; });
//Seg. fault still remains.
add(tTask);
}
};

int main()
{
Console _Console;
_Console.foo();
}


For quick online IDE code test




This should probably be another question, but I think it's too simple.
I've heard before, that if a non-trivial is used in a union, it should be taken care of if the values are switched, how would one do that, and is it necessary if I do not intend to have any value changes on runtime?

EDIT1



Removed the union of vector and std::function

EDIT2



The Seg. Fault is most likely caused by the deconstructor ~Task();.

Answer

If this constructor is called:

Task(std::string const &arg_0, std::function<void(std::string const &)> arg_1)
{ Name = arg_0; Job = arg_1; }

then it crashes for sure when destructor is called because Subtasks is not initialized.

~Task() { for (auto tItem : *Subtasks) { delete tItem; } }

Fix:

Task(std::string const &arg_0, std::function<void(std::string const &)> arg_1)
{ Name = arg_0; Job = arg_1; Subtasks = nullptr;}

~Task() { if (Subtasks!=nullptr) for (auto tItem : *Subtasks) { delete tItem; } }

and (thx for the comments) gather/replace and fix all other constructors like this:

 Task(std::string const &arg_0 = "", std::vector<Task*> *arg_1 = nullptr) { Name = arg_0; Subtasks = arg_1; }

After some discussion, it appears that there's a possible design flaw in the code of the question: deleting the contents of the vector is dangerous since the pointers may be still in use by other clients, or may not even been allocated, but just reference local variables. And why Subtasks is a pointer in the first place? Why not deleting it?

Comments