c++queueinvalid-pointer

Templated Dequeue invalid pointer: Fails on deleting class


My templated queue's dequeue function works fine for a queue of string, but if I use my custom Robot class, it crashes upon trying to delete the pointer. I'm curious as to why.

For example, in main.cpp

#include <iostream>
#include <cstdlib>
#include <cstring>
#include "robotqueue.h"
#include "robotcustomer.h"
#include "servicestation.h"

using namespace std;

int main()
{
    //- TEST ONE: QUEUE<STRING> -//
    RobotQueue < string > stringQueue;
    string a("Tim");
    string b("Greg");

    stringQueue.enqueue(a);
    stringQueue.enqueue(b);
    stringQueue.dequeue();
    stringQueue.dequeue();

    //- TEST TWO: QUEUE<RobotCustomer> -//
    RobotQueue < RobotCustomer > robotQueue;
    RobotCustomer e("Tim",3);
    RobotCustomer f("Greg",5);

    robotQueue.enqueue(e);
    robotQueue.enqueue(f);
    robotQueue.dequeue();            <--- Segfault occurs here
    robotQueue.dequeue();
    return 0;
}

the string queue works fine, but I get this error:

***Error in `q': munmap_chunk(): invalid pointer: 0x0000000001d6c108 ***
Aborted (core dumped)

My templated queue looks about like this (dunno if this ya need more).

robotqueue.hpp

// Default Constructor
template <typename T>
RobotQueue<T>::RobotQueue()
{
    m_size = 0;
    m_front = NULL;
    m_back = NULL;
}
// Default destructor
template <typename T>
RobotQueue<T>::~RobotQueue() 
{
    Node<T>* currNode = m_front, *nextNode = NULL;
    while ( currNode != NULL )
    {
        nextNode = currNode->m_next;
        delete currNode;
        currNode = nextNode;
    }
    m_size = 0;
}

template <typename T>
void RobotQueue<T>::enqueue(const T& x)
{
    Node<T> *newNode = new Node<T>;
    newNode->m_data = x;
    newNode->m_next = NULL;
    if(m_front == NULL)
        m_front = newNode;
    else
        m_back->m_next = newNode;
    m_back = newNode;
    m_size++;                           // Increments queue size
    return;
}

template <typename T>
void RobotQueue<T>::dequeue()
{
    Node<T>* tempNode = new Node<T>;
    if(m_front == NULL)
        cout << "dequeue error: Queue is empty" << endl;
    else
    {
        tempNode = m_front;
        m_front = m_front->m_next;
        delete tempNode;     <-- Segfault occurs here in RobotCustomer class
        m_size--;                       // Increments queue size
    }
    return;
}

I'm assuming it has to do with RobotCustomer being a class so m_data can't point to it or something? Not an expert here :p

RobotCustomer.h

/* ------------------  Class RobotCustomer ------------------ */
class RobotCustomer
{
private:
  string m_name;                // Name of Robot
  string* m_reqServices;        // Array of services requeseted
  int m_numServices;            // Number of services requested
  int m_currService;            // Number of services compelted
  bool m_busy;                  // Logic for if robot is in line/servicing
  bool m_done;                  // Logic for if robot is done
public:
  //- functions and such that I don't think affect the queue -//

Thanks for your time :)

---------------------UPDATED_WITH CONSTRUCTORS/DECONSTRUCTORS-------------------

RobotCustomer.cpp

// Default Constructor
RobotCustomer::RobotCustomer()
{
    m_name = "";
    m_numServices = 0;
    m_currService = 0;
    m_busy = false;
    m_done = false;
}
// Overloaded Constructor
RobotCustomer::RobotCustomer(string n, int x)
{
   m_name = n;
   m_numServices = x;
   m_reqServices = new string[m_numServices]; 
   m_currService = 0;
   m_busy = false;
   m_done = false;
}

// Default Destructor
RobotCustomer::~RobotCustomer()
{
    delete m_reqServices;
}

Solution

  • Lets look at a few of the lines in your function:

    Node<T>* tempNode = new Node<T>;
    tempNode = m_front;
    delete tempNode;
    

    First you allocate memory and assign its pointer to the tempNode variable. Then you reassign that variable to point to some other memory, losing the original pointer. Then you try to free the memory, which is no longer the original memory you allocated.

    This should not cause the crash (as fas as I can see), but it is a memory leak.


    My guess as to what is causing the crash is that you probably allocate memory dynamically in the RobotCustomer constructor, for the m_reqServices member. But if you don't implement a copy-constructor or copy-assignment operator, or just do a shallow copy of the pointer in those functions, then with the assignment

    newNode->m_data = x
    

    in the enqueue function you will have two object with the same pointer, and when one of them is deleted its destructor deletes the allocated memory, leaving the other objects pointer now pointing to unallocated memory, leading to undefined behavior when you try to free it again.

    You need to read about the rules of three, five and zero. My recommendation is to use std::vector or std::array (as applicable) and simply follow the rule of zero.

    Of course, talking about using the standard containers makes me wonder why you don't use std::queue to begin with?