Seemingly random program failure at end of completion C++ [closed]

Question

Closed. This question does not meet Stack Overflow guidelines. It is not currently accepting answers.

---

This question appears to be off-topic because it lacks sufficient information to diagnose the problem. Describe your problem in more detail or include a minimal example in the question itself.

Closed 8 years ago.

Improve this question

Warning: I understand if anyone may want to stop reading now, this post contains ~275 lines of code across 6 files (although nothing very complex)! I realize this is usually a bad thing to do, but it's a last ditch effort as I've put cout's in everything method showing none of them crash or do anything that I wouldn't expect, researched implementation of the standard methods I'm using, and ran this code with a huge variety of inputs but sometimes it runs successfully, other times it fails (after finishing everything). I can't find any pattern or broken code segment.

The program simulates a type of shop with a single server allowing customers to order one of two things and there is a waiting line. The user inputs the simulation length, customer arrival probability per time unit (minute), and the time it takes for each item to be made. After running, the program then prints out a few statistics - total wait time (excluding those remaining in line), total customers served, and average wait time.

Even with long simulations (100,000 minutes) I've seen successful and failed runs (again, only failing after simulation completion). At first I thought it looked like using (>= 1) for arrival probability (customer arrives each minute) always worked, but have since noticed those failing. If anything, it seems fairly high arrival (> ~.8) and very low (<= ~.01) arrival probabilities crash the least often in long simulations, but still can sometimes in short ones. Very odd!

Whenever it does crash, the debugger shows the program counter stopping at the closing brace of queueType's destructor, but this destructor seems extrememly standard to me, and the same syntax has worked with other classes that allocate memory on the heap with their constructors? I feel like the answer must be something fairly basic that is eluding me.

Any help would be greatly appreciated, code follows:

queueType.h:

#ifndef QUEUETYPE_H
#define QUEUETYPE_H
#include <algorithm>
#include <cstdlib>

template<class Type>
class QueueType {
public:
    QueueType();
    ~QueueType();
    QueueType(const QueueType& other);
    Type& getFront() {return queueArray[front];}
    int getNumElements() const {return numElements;}
    void reposition();
    void addElement(Type);
    bool isEmpty() const {return numElements == 0;}
    bool isFull() const {return SIZE == numElements;}
    void updateWaitTimes(Type*&, int&, int&);

    QueueType<Type>& operator=(const QueueType other);

    friend void swap(QueueType& first, QueueType& second) {
        using std::swap;
        swap(first.front, second.front);
        swap(first.back, second.back);
        swap(first.numElements, second.numElements);
        swap(first.queueArray, second.queueArray);
    }
private:
    static const int SIZE = 25;
    int front, back, numElements;
    Type *queueArray;
};

template<class Type>
QueueType<Type>::QueueType() {
    queueArray = new Type[SIZE];
    front = back = numElements = 0;
}

template<class Type>
QueueType<Type>::~QueueType() {
    delete [] queueArray;
}

template<class Type>
QueueType<Type>::QueueType(const QueueType& other):
        queueArray(new Type[SIZE]),
        front(other.front),
        back(other.back),
        numElements(other.numElements)
{
    std::copy(other.queueArray, other.queueArray + SIZE, queueArray);
}

template<class Type>
void QueueType<Type>::reposition() {
    front = (front + 1) % SIZE;
    back = (back + 1) % SIZE;
    numElements--;
}

template<class Type>
void QueueType<Type>::addElement(Type newElement) {
    if (isEmpty()) {
        queueArray[0] = newElement;
        front = back = 0;
        numElements = 1;
    } else {
        back = (back - 1) % SIZE;
        queueArray[back] = newElement;
        numElements++;
    }
}

template<class Type>
void QueueType<Type>::updateWaitTimes(Type*& element, int& position, int& counter) {
    if (isEmpty()) {
        element = NULL;
    } else {
        if (position == 0) {
            position = front;
        }
        element = &queueArray[position];
        position = (position + 1) % SIZE;
    }
    if (counter == numElements) {
        element = NULL;
    }
    counter++;
}

template<class Type>
QueueType<Type>& QueueType<Type>::operator=(const QueueType other) {
    swap(*this, other);
    return *this;
}
#endif  /* QUEUETYPE_H */

customerType.h:

#ifndef CUSTOMERTYPE_H
#define CUSTOMERTYPE_H

class CustomerType {
public:
    CustomerType();
    CustomerType(int, int);
    ~CustomerType();
    CustomerType(const CustomerType& other);
    void incrementWaitTime() {waitTime++;}
    int getArrivalTime() const {return arrivalTime;}
    int getWaitTime() const {return waitTime;}

    CustomerType& operator=(const CustomerType& other);
private:
    int ID, arrivalTime, waitTime;
};

#endif  /* CUSTOMERTYPE_H */

customerType.cpp:

#include "customerType.h"

CustomerType::CustomerType() {
    waitTime = arrivalTime = ID = 0;
}

CustomerType::CustomerType(int arrivalTime, int ID) {
    this->arrivalTime = arrivalTime;
    this->ID = ID;
    waitTime = 0;
}

CustomerType::~CustomerType() {
}

CustomerType::CustomerType(const CustomerType& other) {
    waitTime = other.waitTime;
    arrivalTime = other.arrivalTime;
    ID = other.ID;
}

CustomerType& CustomerType::operator=(const CustomerType& other) {
    waitTime = other.waitTime;
    arrivalTime = other.arrivalTime;
    ID = other.ID;
    return *this;
}

serverType.h:

#ifndef SERVERTYPE_H
#define SERVERTYPE_H

#include "customerType.h"
#include <cstdlib>
#include <string>

class serverType {
public:
    serverType();
    ~serverType();
    serverType(const serverType& other);
    bool isFree() const {return (status == "free");}
    void setCustomer(CustomerType& newCustomer, int& transactionTime);
    void decrementTransactionTime();

    serverType& operator=(const serverType& other);
private:
    std::string status;
    int transactionTime;
    CustomerType currentCustomer;
};

#endif  /* SERVERTYPE_H */

serverType.cpp:

#include "serverType.h"

serverType::serverType() {
    status = "free";
    transactionTime = 0;
}

serverType::~serverType() {
}

serverType::serverType(const serverType& other) {
    status = other.status;
    transactionTime = other.transactionTime;
    currentCustomer = other.currentCustomer;

}

void serverType::setCustomer(CustomerType& newCustomer, int& transactionTime) {
    currentCustomer = newCustomer;
    this->transactionTime = transactionTime;
    status = "busy";
}

void serverType::decrementTransactionTime() {
    transactionTime--;
    if (transactionTime == 0)
        status = "free";
}

serverType& serverType::operator=(const serverType& other) {
    status = other.status;
    transactionTime = other.transactionTime;
    currentCustomer = other.currentCustomer;
    return *this;
}

main.cpp:

#include "queueType.h"
#include "serverType.h"
#include <ctime>
#include <climits>
#include <iostream>
using namespace std;

int main(int argc, char** argv) {

    int simulationTime, coneTime, shakeTime, currentTime = 0;
    int customerID = 1, totalWaitTime = 0, customersServiced = 0;
    double arrivalProb;

    cout << "Time-driven ice cream shop simulation" << endl
            << "Enter the following information to begin:" << endl << endl;
    cout << "Length of simulation (in minutes): ";
    cin >> simulationTime;
    cout << endl << "Probability of customer arrival each minute (example: 0.25): ";
    cin >> arrivalProb;
    cout << endl << "Minutes to make an ice cream cone: ";
    cin >> coneTime;
    cout << endl << "Minutes to make a shake: ";
    cin >> shakeTime;
    cout << endl << endl;

    QueueType<CustomerType> Line;
    serverType server;
    float chance;
    srand(time(0) % INT_MAX);

    while (currentTime < simulationTime) {
        chance = float (rand())/RAND_MAX;
        if (chance < arrivalProb) {
            if (!Line.isFull()) {
                Line.addElement(CustomerType(currentTime, customerID));
                customerID++;
            } else {
                cout << "Customer #" << customerID 
                        << " came during a full line and left!" << endl;
                customerID++;
            }
        }
        if (server.isFree() && (!Line.isEmpty())) { //going with 40% shake, 60% cone
            customersServiced++;
            if (chance < 0.4) {
                server.setCustomer(Line.getFront(), shakeTime);
            } else {
                server.setCustomer(Line.getFront(), coneTime);
            }
            totalWaitTime += Line.getFront().getWaitTime();
            Line.reposition();
        } else if (!server.isFree()) {
            server.decrementTransactionTime();
            CustomerType *customerPointer = new CustomerType();
            int position = 0, counter = 0;
            Line.updateWaitTimes(customerPointer, position, counter);
            while (customerPointer != NULL) {
                (*customerPointer).incrementWaitTime();
                Line.updateWaitTimes(customerPointer, position, counter);
            }
            delete customerPointer;
        }
        currentTime++;
    }
    cout << endl << endl << "Simulation complete." << endl << endl;
    cout << "Total wait time: " << totalWaitTime << endl
            << "Customers serviced: " << customersServiced << endl
            << "Average wait time: " << float (totalWaitTime) / customersServiced;
    return 0;
}

Note that the queueType copy constructor/overloaded =/destructor never getting called until the destructor does once in the very end. I also know I don't need to have a customerType (currentCustomer) as one of serverType's private members, but just for the sake of realism.

Answer 1

WOW. Just finally realized the reason it was crashing was how I changed back around in queueType::reposition and queueType::addElement, in reposition I didn't need to move back at all since it's just called after someone leaves the front, and in my add I meant to move back BACK one but used - not + and moved it forward...program fixed. Thank you for answers/comments

Answer 2

You are mismanaging memory here:

CustomerType *customerPointer = new CustomerType();
int position = 0, counter = 0;
Line.updateWaitTimes(customerPointer, position, counter);

You are allocating memory for customerPointer. Then you change the value of what customerPointer points to in the Line.updateWaitTimes function. Then you do this:

delete customerPointer;

So what you allocated and what you deleted have different values. You're corrupting the heap by attempting to delete an address that doesn't start at the beginning of the dynamically allocated block.

If what you are deleting is a pointer to dynamically allocated memory, i.e. you designed it this way, but is a "different" pointer than the original you started out with, then you need to rewrite your code so you're not doing this "pointer dance" between customerPointer and the Line.updateWaitTimes function.

This is just one of probably many issues with your code you need to fix. One fix is to quit with the manual memory mamagement within your main() function. Learn to write code that minimizes or eliminates the usage of raw naked pointers. Yes, your QueueType class must do memory management, sure, but that doesn't mean your main() has to do this also.

Also, your QueueType class maintains its own memory that should not be fooled around with by an outside entity. Look at your QueueType::updateWaitTimes function -- why is it giving a pointer to the passed in "element" pointer? You then use this pointer to your internal queue and finagle with it in main(), which gives disastrous results. Writing code like this means that you haven't totally grasped the meaning of "encapsulation".

Answer 3

This line likely as problem, as it can leave back as negative

back = (back - 1) % SIZE;

you probably meant something like

back = (SIZE + back - 1) % SIZE;