Search code examples
c++c++11memory-leaksvisitor-pattern

Why do these visit methods cause memory leaks?


I am working on a medium sized C++ framework making use of the visitor pattern.

A valgrind test of a program implementing this framework reported a number of memory leaks that could be tracked down to one of the visitors, namely the copyCreator.

template<typename copyNodeType>
struct copyCreator {
    copyCreator {}
    copyCreator(node * firstVisit) {
        firstVisit->accept(*this);
    }

    ~copyCreator() {
        copy.reset();
        for(auto ptr : openList) {
            delete ptr;
        }
    }

    std::unique_ptr<copyNodeType> copy = 0;
    vector<nonterminalNode *> openList;

    // push to tree
    template<typename nodeType>
    void push(nodeType * ptr) {
        if (copy) {
            // if root is set, append to tree
            openList.back()->add_child(ptr);
        }
        else {
            auto temp = dynamic_cast<copyNodeType *>(ptr);
            if(temp) {
                copy = std::unique_ptr<copyNodeType>(temp);
            }
        }
    }

// ...

    void visit(struct someNonterminalNode & nod) {
    auto next = new someNonterminalNode(); //This is leaked
    push(next);
    openList.push_back(next);
    nod.child->accept(*this);
    openList.pop_back();
};

There are a two main reasons why I am confused about this:

  • The two different constructors cause a different number of leaks
  • The leaks are reported to occur during visits

The accept methods of all nodes simply triggers a standard double dispatch to the visit method of the correct visitor.

I am fairly new to C++ programming and might have overlooked some really fundamental issue.


Solution

  • copyCreator<nodeType>::push(ptr) is supposed to take ownership of ptr. But it fails to do so if (a) ptr is not of type nodeType* (as determined by dynamic_cast), and (b) no node of type nodeType has been visited yet.

    In other words, copyCreator<nodeType> creates, and promptly leaks, copies of all nodes until it encounters one of type nodeType.

    This is precisely what happens in copyCreator<programNode> cpy2(&globalScope, a);, where a is forallNode*. cpy2 expects to encounter programNode (which it never does), and meanwhile, it copies and leaks all other nodes.