Safe to delete parent of vector of pointers when those pointers have been shared?

Question

I'm getting memory leak issues in one of my applications that I'm going through and trying to fix. One of my suspected problem points is where I parse lines from a file into commands using BNFC:

void LineStreamScriptProvider::populateQueue()
{
    if(shouldPopulate())
    {
        TasScript::ShowAbsyn shower;
        std::string line;
        while(queueSize() < 30 && !stream.eof())
        {
            std::getline(stream, line);
            const char* lineCStr = line.c_str();
            TasScript::Program* lineCmds = TasScript::pProgram(lineCStr);
            TasScript::P* asLeaf = static_cast<TasScript::P*>(lineCmds);
            TasScript::ListCommand* cList = asLeaf->listcommand_;
            for_each(cList->begin(), cList->end(), [this, shower](TasScript::Command* cmd) {
                // log_to_sd_out("Parsed command %s\n", shower->show(cmd));
                std::shared_ptr<TasScript::Command> cmdShared(cmd);
                pushToQueue(cmdShared);
            });
        }
        if(stream.eof())
        {
            afterEOF();
        }
    }
}

For reference:

class P : public Program
{
public:
  ListCommand *listcommand_;
// ...

class ListCommand : public Visitable, public std::vector<Command*>
{
// ...

BNFC constructs these with new and then returns the pointers. Is it safe to delete lineCmds without deleting the value held by cmdShared?

Answer 1

Sorry, I wasn't aware of BNFC and that it creates raw pointers for you.

Is it safe to delete lineCmds without deleting the value held by cmdShared?

If you are creating a shared pointer from a raw pointer the shared pointer takes ownership of that resource. The shared pointer will maintain a reference count for that resource until it drops to zero, i.e. when all shared pointers for that resource go out of scope, at which point it will try to destroy the resource.

Here you are creating a shared pointer and passing it to a queue:

std::shared_ptr<TasScript::Command> cmdShared(cmd);
pushToQueue(cmdShared);

The shared pointer will take care of the memory management for you. So there is no need to explicitly call delete on the vector of commands. Once all shared pointers of type std::shared_ptr<TasScript::Command> are destroyed the resource is also destroyed.

So no, it is not safe to delete a vector of pointers in this case.

---

Another simpler solution is to take a copy of the TasScript::Command:

TasScript::Command cmdCopy(*cmd);

Then change pushToQueue() to accept by const TasScript::Command&, not by shared pointer. Then you don't have to worry about the pointer being destroyed since you have a copy of the value.

It looks like in your while loop you are leaking memory. Don't you have to delete lineCmds and cList?