Valgrind error when removing items from a vector

This might look like a duplicate to most of you. But I spent so much time finding a fix for this. Implemented many solutions given in stackoverflow and other coding sites. Finally I managed to fix it but still I'm clueless of what was wrong with my older implementation.

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

Note:

• I was testing my code from unit tests (google test framework).
• Compiled using C++11

Test:

• Queue has 2 SAPA items (created by new operator)
• Deleting the firstitem by its id (Queue has only one now)
• Deleting the only item left by its id
• Second delete seems to give the valgrind error when accessing the m_id_ of the item

Here is my Queue Item base class

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

  virtual ~Item() {}

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

Here is the child class

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

Old code used to remove item if it meets a certain criteria (RemoveIf). Caused the VALGRIND issues.

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

std::vector<Item*>::iterator it = m_queue_.begin();
  while (it != m_queue_.end()) {
    network::multiplexer::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 says invalid read of size 8. Sorry this contains some project specific 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)

Below FIXED the valgrind issues This is the new code which iterates backwards 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);
    }
  }