从vector中移除项目时出现错误

Valgrind error when removing items from a vector

本文关键字：错误 项目 vector      更新时间：2023-10-16

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

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

注意:

  • 我正在测试我的代码从单元测试(谷歌测试框架)。
  • 使用c++ 11编译
  • m_queue_是std::vector
  • 使用的Google c++编码标准

测试:

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

这是我的Queue Item基类 

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)。

在许多帖子中，这被提议作为从向量中删除项的正确方法。

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);
    }
  }

EDIT

经你方澄清，实际原因似乎很清楚了。

你正在传递"id"作为const string&，它传递对原始对象的引用而不是副本。因为它来自第一个对象，我猜上面的某个地方有const string& id = *m_queue_.begin()->m_id_;，然后你继续把它传递给RemoveIf。因为比较保证删除第一个元素，所以这在循环中发生。现在id是对该项中的数据的悬空引用。它在反向迭代版本中工作的原因是，第一项成为您删除的最后一项。在您删除它之后，没有其他东西会查看id。您可能会更改将id赋值为string id = *m_queue_.begin()->m_id_;的代码行。通过将其设置为string而不是string&，您可以强制进行复制。该副本的生存期将持续到作用域结束。

结束编辑

您应该注意的一件事是std库函数。特别是，您需要vector::erase()和std::remove_if()。你想要的erase函数的版本是接受一对迭代器，而不是每次擦除一个迭代器。当你调用它时，它看起来像m_queue_.erase(XXXX, m_queue_.end())。现在XXXX是什么-它最终是remove_if的返回值。remove_if的参数是一对迭代器和一元谓词。(即一个函数，无论在你的向量和返回true，如果它应该被删除的const T&。)返回值是位于未删除项末尾的迭代器。

在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)));

如果vector包含nullptr或其他不应该解引用的值是有效的，则将这些检查添加到谓词函数中。此外，如果向量拥有这些项，您可能希望将其设置为vector<std::unique_ptr<Item>>而不是vector<Item*>，如果不这样做，则使用erase(remove_if)习惯用法可能会泄漏。它也节省了你需要记住删除的东西。

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

参考:

  • https://en.wikipedia.org/wiki/Erase-remove_idiom
  • 我什么时候使用哪一种指针?
相关文章：