对象创建期间的资源泄漏答案

【问题标题】：Resource leak during object creation对象创建期间的资源泄漏
【发布时间】：2011-04-12 02:00:08
【问题描述】：

我有以下代码用于在图中创建节点。运行静态检查工具（覆盖率）时出现资源泄漏错误。如果您能指出如何改进代码，我将不胜感激：

class node {   
   public :  
     explicit node(std::string& name) : m_name(name) { }  
     void setlevel(int level)  
     { m_level = level; }  
   private :    
     ...  
 }  
class graph {  
   public :  
      void populateGraph()  
      {  
         std::string nodeName = getNodeName();   
         /* I get error saying variable from new not freed or pointed-to in function  
            nc::node::node(const std::string...) */  
         node* NodePtr = new node(nodeName);  
         /* I get error saying variable NodePtr not freed or pointed-to in function  
            nc::node::setLevel(int) */   
         NodePtr->setLevel(-1);  
         if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())  
             m_name2NodeMap[nodeName] = NodePtr;  
         NodePtr = NULL;  
      }  
....  
private :  
  std::map< std::string, node*> m_name2NodeMap;   
}

我以为我需要在populateGraph 中添加delete NodePtr，但随后发布它会调用节点析构函数（~node）并从图中删除节点。所以，我设置了NodePtr=NULL 看看它是否有帮助，但它没有。

【问题讨论】：

每个新的都需要删除，而您在任何地方都没有删除。如果编译器为 Node 对象分配了内存，编译器会清理它。但如果你明确使用“new”，则需要使用“delete”进行清理。在我看来，您应该在创建之前检查 nodeName 是否存在于您的地图中。您的图形类应该有一个解构器，它将在 m_name2NodeMap 中的每个元素上调用 delete。
@Apprentice 队列：不，每个new 都不需要delete。每个new 结果应存储在资源管理器（智能指针或其他任何东西）中，该管理器将在调用时处理内存。尝试手动操作类似于在火药桶中使用火炬。
@Matthieu M.，我认为在这里使用“资源管理器”有点矫枉过正，并且黑箱化了应该释放分配的基本思想。
@Apprentice Queue：不是。 unique_ptr、scoped_ptr 或 auto_ptr 绝不是矫枉过正。它们很简单，运行良好，而且与手写代码不同，它们不会搞砸。 delete 是一种代码气味（或“代码预兆”，一种阻碍厄运的警告）。 RAII 是唯一可靠的解决方案，任何选择不可靠解决方案的人要么是恶意的，要么是无知的。我希望后者能被击退。

标签： c++ pointers graph coverity-prevent

【解决方案1】：

我不熟悉覆盖率或它使用的确切规则，但如果节点的名称已经在映射中，您似乎会发生内存泄漏。也就是说，如果你的 if 语句的主体没有被执行，那么你就会失去指向你刚刚分配的内存的指针。也许你想要这样的东西：

if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())  
    m_name2NodeMap[nodeName] = NodePtr;  
else
    delete NodePtr;
NodePtr = NULL;

编辑：由于我和 Daemin 几乎同时回复，让我添加更多细节：

正如 ildjarn 所提到的，您还需要通过添加析构函数来释放那些最终出现在地图中的对象：

~graph()
{
    for( std::map< std::string, node*>::iterator i = m_name2NodeMap.begin(); 
         i != m_name2NodeMap.end(); ++i )
    {
        delete i->second;
    }
}

为了完整起见，我应该指出：

析构函数完成后映射将被自动删除，因为它是一个成员变量。
节点映射中的条目会在映射删除时自动删除。
删除条目时将删除字符串键。

处理复杂对象生命周期的首选方法是使用智能指针。例如，boost::shared_ptr 或 tr1::shared_ptr 将像这样工作。注意：我可能没有确切的语法。

class node {   
    ...
}

class graph {  
    public :  
    void populateGraph()  
    {  
        std::string nodeName = getNodeName();   
        boost::shared_ptr< node > NodePtr( new node(nodeName) );
        NodePtr->setLevel(-1);  
        if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())  
            m_name2NodeMap[nodeName] = NodePtr;
    }  
    ....  
    private :  
        std::map< std::string, boost::shared_ptr<node> > m_name2NodeMap;   
    }
};

看看我们是如何消除析构函数和显式调用删除的？现在节点对象将像节点名称一样自动销毁。

在另一个节点上，您应该查看 std::map::insert 函数，该函数应该会一起消除该 if 语句。

【讨论】：

感谢您的回答。我有一个关于“2.删除地图时节点地图中的条目将被自动删除”的问题。通过条目，您的意思是每个元素都被删除了。如果是这样，为什么每个元素中的节点*没有被删除。例如。地图[“A”] = AddrofObjA。假设每个节点Obj占用50个字节，不会删除map中的条目“A”，删除从AddrofObjA开始的50个字节吗？谢谢
共享指针仅供专家使用。共享所有权极难正确管理（无论是语义上还是技术上），只能作为最后的手段。哦，如果需要析构函数，还需要复制构造函数和赋值运算符。
@srikrish：您在地图中存储的是指向节点的指针。该映射仅存储密钥“A”和内存地址。当您从映射中删除元素时，它会释放与键和地址关联的内存。但是，它不会自动删除该地址中存储的任何内容。注意：我忽略了密钥存储在 std::string 对象中的问题，该对象在内存管理方面做了一些有趣的事情，但它给人一种错觉，即密钥只是存储在元素中并被删除。
@Matthieu：我不同意共享指针是“最后的手段”。虽然它们确实有诸如循环引用问题之类的警告，但我想说它们使用起来更安全，并且比编写自己的析构函数更容易证明其正确性。特别是，如果你想做的是存储指针，那么我认为共享指针更好。
我没有建议编写你自己的析构函数（正如我所说的那样，充其量是不一致的，除非你也编写或禁用复制和赋值）。我想指出，许多人将shared_ptr 视为防止内存泄漏的灵丹妙药。他们不是。有许多更简单的选择。如果你想要一个拥有多态值的地图，惯用的解决方案是boost::ptr_map<Key,Value>。您可以在 C++0x 中使用 std::map<Key, std::unique_ptr<Value>> 模拟它，但它不提供深度复制，并且像 ptr_map 那样提供一些迭代器糖衣。

【解决方案2】：

你需要做的是给graph一个析构函数，在它里面，deletem_name2NodeMap中的所有node*s。当然，因为需要析构函数，所以还需要复制构造函数和复制赋值操作符，否则肯定会导致内存损坏。

对于if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end()) 到delete NodePtr;，您还需要一个else 子句。

【讨论】：

【解决方案3】：

当您不将 NodePtr 添加到列表时，您并没有释放它。 if 语句需要一个 else，你 delete NodePtr;

if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())
{
    m_name2NodeMap[nodeName] = NodePtr;
}
else
{
    delete NodePtr;
}
NodePtr = NULL;

【讨论】：

【解决方案4】：

其他人已经涵盖了泄漏的问题。事实上有很多漏洞，所以我什至不会费心评论它们......（至少populateGraph，~Graph，Graph(Graph const&)和Graph& operator=(Graph const&)......）

我更喜欢提供一个简单行之有效的解决方案：

class Graph
{
public:
  void addNode(std::string name) {
    _nodes.insert(std::make_pair(name, Node(name));
  }

private:
  std::map<std::string, Node> _nodes;
};

这是怎么回事？

动态内存分配是不必要的，map 可以完美地包含Node 的值，这样不会有任何泄漏。
std::map::insert 只会在没有等效键存在的情况下执行插入，不需要执行 find + [] （这是两倍复杂，因为你计算了存储元素的位置的两倍）

【讨论】：