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