从向量中删除项目时出现 Valgrind 错误

Question

对于大多数人来说，这可能看起来像是重复的。但是我花了很多时间来解决这个问题。实现了 stackoverflow 和其他编码站点中给出的许多解决方案。最后我设法修复了它，但我仍然不知道我的旧实现有什么问题。

请帮助我找出导致我的旧代码、新代码、单元测试和 valgrind 错误的确切错误的原因。

注意：

• 我正在通过单元测试（Google 测试框架）测试我的代码。
• 使用 C++11 编译
• m_queue_ 是一个 std::vector
• 使用的 Google C++ 编码标准

测试：

• 队列有 2 个 SAPA 项（由新操作员创建）
• 按 id 删除第一个项目（队列现在只有一个）
• 删除唯一的项目 由它的 id 留下
• 在访问项目的 m_id_ 时，第二次删除似乎给出了 valgrind 错误

这是我的队列项目基类

class Item {
 public:
  Item() {
    type = Type::kInvalid;
  }

  virtual ~Item() {}

  Type type;
  string m_id_ = string("");
};

这是子类

class SAPA : public Item {
 public:
  SAPA() { Item::type = Type::kSAPA; }
  ~SAPA() {}
};

旧代码用于在满足特定条件时删除项目 (RemoveIf)。导致 VALGRIND 问题。

This was proposed as a correct way to remove items from a vector in many posts.

void Queue::RemoveItems(const string& id) const {
  vector<Item*>::iterator it = m_queue_.begin();
  while (it != m_queue_.end()) {
    Item* item = *it;
    if (item == nullptr) {
      continue;
    }

    if (RemoveIf(item, id)) {
      delete item;
      item = nullptr;
      it = m_queue_.erase(it);
    } else {
      ++it;
    }
  }
}

RemoveIf 函数

bool Queue::RemoveIf(Item* item,
                     const string& id) const {
**cout << id.c_str() << endl; <--- seems to cause the invalid read**

  if (item->m_id_.compare(id) == 0) {
    return true;
  }

  return false;
}

VALGRIND 输出显示大小为 8 的无效读取。抱歉，其中包含一些项目特定名称。

> ==21919== Invalid read of size 8
> ==21919==    at 0x5880B90: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const (in
> /usr/lib64/libstdc++.so.6.0.21) 
> ==21919==    by 0xEC416C: Queue::RemoveIf(network::multiplexer::Item*, blf::String const&) const (network_multiplexer_queue.cc:99)
> ==21919==    by 0xEC3FFB: Queue::RemoveItems(blf::String const&) const (network_multiplexer_queue.cc:85)
> ==21919==    by 0xEC4FDC: Queue::OnTimer() const (network_multiplexer_queue.cc:228)
> ==21919==    by 0xFB05E0: (anonymous namespace)::NetworkMultiplexerTest_sapaTimeout_shouldBeHandled_successfully_Test::TestBody()
> (network_multiplexer_comm_test.cc:1201)
> ==21919==    by 0x1232186: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x122C547: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x12124B7: testing::Test::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1212D99: testing::TestInfo::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1213444: testing::TestCase::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1219F2E: testing::internal::UnitTestImpl::RunAllTests() (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1233583: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
> bool>(testing::internal::UnitTestImpl*, bool
> (testing::internal::UnitTestImpl::*)(), char const*) (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==  Address 0x6d24a00 is 16 bytes inside a block of size 112 free'd
> ==21919==    at 0x4C2A131: operator delete(void*) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==21919==    by 0xED3991: SAPA::~SAPA() (network_multiplexer_queue_item.h:82)
> ==21919==    by 0xEC4045: Queue::RemoveItems(blf::String const&) const (network_multiplexer_queue.cc:86)
> ==21919==    by 0xEC4FDC: OnTimer() const (network_multiplexer_queue.cc:228)
> ==21919==    by 0xFB05E0: (anonymous namespace)::NetworkMultiplexerTest_sapaTimeout_shouldBeHandled_successfully_Test::TestBody()
> (network_multiplexer_comm_test.cc:1201)
> ==21919==    by 0x1232186: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x122C547: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x12124B7: testing::Test::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1212D99: testing::TestInfo::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1213444: testing::TestCase::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1219F2E: testing::internal::UnitTestImpl::RunAllTests() (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)
> ==21919==    by 0x1233583: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
> bool>(testing::internal::UnitTestImpl*, bool
> (testing::internal::UnitTestImpl::*)(), char const*) (in
> /home/sajith/cioffi/cioffi-linux/build/unit_tests)

修复了以下 valgrind 问题 这是向后迭代并删除项目的新代码。

auto it = m_queue_.end();
  while (it > m_queue_.begin()) {
    it--;
    Item* item = *it;
    if (item == nullptr) {
      continue;
    }

    if (RemoveIf(item, id)) {
      delete item;
      item = nullptr;
      it = m_queue_.erase(it);
    }
  }

Answer 1

编辑

经过你的澄清，真正的原因似乎很清楚了。

您将“id”作为const string& 传递，它传递对原始对象的引用而不是副本。因为它来自第一个对象，我猜你上面的某个地方有const string& id = *m_queue_.begin()->m_id_;，然后你继续将它传递给RemoveIf。因为保证比较会删除第一项，所以这发生在您的循环中。现在id 是对该项目中数据的悬空引用。它在您的反向迭代版本中起作用的原因是第一项成为您删除的最后一项。删除它后，id 没有其他内容。您可能可以将分配id 的代码行更改为string id = *m_queue_.begin()->m_id_;。通过在此时将其设为string 而不是string&，您将强制制作副本。该副本将有一个生命周期，直到范围结束。

结束编辑

您应该关注的一件事是标准库函数。特别是，您需要vector::erase() 和std::remove_if()。您想要的擦除版本是采用一对迭代器而不是一次擦除一个迭代器的版本。当你调用它时，它看起来像m_queue_.erase(XXXX, m_queue_.end())。现在什么是 XXXX - 最终成为 remove_if 的返回值。 remove_if 的参数是一对迭代器和一个一元谓词。 （即，一个函数将 const T& 用于向量中的任何内容，如果应该删除它，则返回 true。）返回值是一个迭代器，就在未删除项目的末尾。

在 c++11 中，这可能看起来像：

string id = "the_id_to_filter";
m_queue_.erase(std::remove_if(m_queue_.begin(), m_queue_.end(), 
                              [&id](const Item* item) { 
                                  return item_.m_id_ == id;
                              });

在 c++11 之前的版本中，您可以将 lambda 替换为以下内容：

struct Filter {
   Filter(const string& id) : id_(id) {}
   string id_;
   bool operator()(const Item* item) { return item.m_id_ == id_; }
};

然后你的电话看起来像：

string id = "the_id_to_filter";
m_queue_.erase(std::remove_if(m_queue_.begin(), m_queue_.end(), Filter(id)));

如果您的向量包含 nullptr 或您不应该取消引用的其他值是有效的，请在谓词函数中添加这些检查。此外，如果向量拥有这些项目，您可能希望将其设为 vector<std::unique_ptr<Item>> 而不是 vector<Item*>，如果您不这样做，那么使用 erase(remove_if) 惯用语可能会泄漏。它还使您无需记住删除内容。

使用库函数将使您免于因循环中的一个错误和各种其他痛苦而感到沮丧。

供参考：

• https://en.wikipedia.org/wiki/Erase-remove_idiom
• Which kind of pointer do I use when?