"Potential memory leak" with std::function

user_1282773 0
Viewed 348

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.

clang-tidy c++
1 Answers

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.

user_17746670
Related