Search code examples
c++clang-tidy

"Potential memory leak" with std::function


Consider this example:

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

using closure_type = std::function<void(void)>;
using closure_vec = std::vector<closure_type>;

class callbacks {
    static closure_type common(std::string name, uint32_t number) {
        return [number, name]() { std::cout << name << number << std::endl; };
    }
public:
    static closure_type foo(uint32_t number) { return common("foo ", number); }
    static closure_type print(std::string msg) {
        return [msg]() { std::cout << "print " << msg << std::endl; };
    }
};

template <typename... calls_t> closure_vec wrap(uint32_t number, calls_t &&... calls) {
    return closure_vec {
        callbacks::foo(number),
        std::forward<calls_t>(calls)...,
    };
}

int main() {
    auto vec = wrap(42,
        callbacks::print("hello, "),
        callbacks::print("world"));

    for(auto &e: vec)
        e();
    return 0;
}

Demo (On the right most tab there is a full message)

When this code is checked with clang-tidy, I get the following warning:

warning: Potential memory leak [clang-analyzer-cplusplus.NewDeleteLeaks]

The line number points at the scope exit of the wrap function.

As I understand the message, the tool is concerned that the results form callbacks::foo might be lost. But I don not understand how is it possible: std::function is a safe class and should destroy everything nicely in its destructor. Also its lifetime is controlled by the vector which is safe too.

What is going on here? How do I fix this or workaround?

Unfortunately I cannot just suppress the warning, as this code is scattered everywhere in the codebase.


Solution

  • Try

    closure_vec retval;
    retval.reserve(sizeof...(calls)+1);
    retval.push_back(callbacks::foo(number));
    ( retval.push_back(std::forward<calls_t>(calls)), ... );
    return retval;
    

    this avoids the const initializer_list contained copies of std function your code created, so should be more efficient as well.

    Live example.

    I tried using a C style array here, but I got the warning as well despite not using a std::initializer_list.

    This also works:

    std::array<closure_type, sizeof...(calls)+1> tmp ={
        nullptr,
        std::forward<calls_t>(calls)...
    };
    tmp[0] = callbacks::foo(number);
    return {std::make_move_iterator(std::begin(tmp)), std::make_move_iterator(std::end(tmp))};
    

    the problem is callbacks::foo(number) within the initalization.