Valgrind error while deleting elements from vector

It may look like a duplicate for most of you. But I spent so much time fixing it. Implemented many solutions provided at stackoverflow and other coding sites. Finally, I managed to fix it, but still I donโ€™t know what was wrong with my old implementation.

Please help me find out what caused the exact error by looking at my old code, new code, unit test and valgrind error.

Note:

  • I tested my code from unit tests (Google testing platform).
  • Compiled using C ++ 11
  • m_queue_ is std :: vector
  • Google C ++ Coding Standards Used

Test:

  • There are 2 SAPA elements in the queue (created by a new operator)
  • Deleting the first element by its identifier (the queue has only one)
  • Removing a single item to the left of its id
  • The second delete seems to give a valgrind error when accessing the m_id_ of the element

Here is my base class Item Item [

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

  virtual ~Item() {}

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

Here is a child class

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

Old code used to remove an item if it meets certain criteria (RemoveIf). Caused problems VALGRIND.

This has been suggested as the correct way to remove elements 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 Function

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 output indicates an invalid read of size 8. Sorry, this contains some specific project names.

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

Issues Issued Below FIXED This is new code that iterates back and removes items.

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);
    }
  }
+4
source share
1 answer

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&, . .

END EDIT

, , - std. , vector::erase() std::remove_if(). , , , , . , m_queue_.erase(XXXX, m_queue_.end()). XXXX - , remove_if. remove_if - . (I.e. , const T& true, .) .

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

pre-++ 11 - :

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*>, (remove_if), , , . , .

, .

:

+3

Source: https://habr.com/ru/post/1658900/


All Articles