Recursive function crashing progam

[N3wb]

I'm having trouble with a recursive function crashing my program. I'll post the function and then explain what it does:

void MainWindow::sendGlobalCoordinates()
{
    if(myThread->TXFlag == 1)
    {
        return sendGlobalCoordinates(); // try try again
    }
    else // TXFlag == 0
    {
        if(globalCoordinatesSent == 0) // if x&y not sent, Send the x
        {
            myThread->TXBytes[0] = 56;   // setGlobalPosX
            myThread->TXData = (int)(pointVector[calPoint2].gX * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 1) // If x sent, but y not sent, sent the y
        {
            myThread->TXBytes[0] = 57;   // setGlobalPosY
            myThread->TXData = (int)(pointVector[calPoint2].gY * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 2) // if x&y sent
        {
            startButton->setEnabled(1); // let the drilling begin!
            return; // the recursive loop must be broken.
        }
        return sendGlobalCoordinates(); // keep going
    }
}

void MainWindow::sendGlobalCoordinates()
{
    if(myThread->TXFlag == 1)
    {
        return sendGlobalCoordinates(); // try try again
    }
    else // TXFlag == 0
    {
        if(globalCoordinatesSent == 0) // if x&y not sent, Send the x
        {
            myThread->TXBytes[0] = 56;   // setGlobalPosX
            myThread->TXData = (int)(pointVector[calPoint2].gX * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 1) // If x sent, but y not sent, sent the y
        {
            myThread->TXBytes[0] = 57;   // setGlobalPosY
            myThread->TXData = (int)(pointVector[calPoint2].gY * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 2) // if x&y sent
        {
            startButton->setEnabled(1); // let the drilling begin!
            return; // the recursive loop must be broken.
        }
        return sendGlobalCoordinates(); // keep going
    }
}

To copy to clipboard, switch view to plain text mode 

OK, so here's the deal. I have a QThread running - named myThread - and its sole purpose is to send and receive data over the serial port. The function you see above was written to solve a problem. I need to send two sets of data over the serial port, but the second set of data can't be sent until the first one has been sent. So my idea was to write this recursive function which first sends the first set of data (if nothing is currently being sent over the serial port) and then, after that, sends the second set of data. If the serial port is busy sending data then the function just returns calling itself and eventually the job should get done. Brilliant idea, right? Unfortunately, the program crashes when I run it, and I know that the recursion is causing the problem (commenting out " return sendGlobalCoordinates();" keeps the program from crashing).

Any advice as to what I should do?

[SixDegrees]

What's the return value of a void function?

[Timoteo]

What's the return value of a void function?

The answer is: nothing, which is what a function with a void return type expects. Now for your question: what does this have to do with the question?

To the OP: Can you show more code? Like the complete definition of MainWindow?

[tbscope]

I need to send two sets of data over the serial port, but the second set of data can't be sent until the first one has been sent.

Use a queue.

[N3wb]

To the OP: Can you show more code? Like the complete definition of MainWindow?

Sure, I can post more code. I'm not sure what's relevant to the problem though and there's a couple thousand lines in the program, too long to post on here. But here's the definition of MainWindow:

#ifndef MAINWINDOW_H
#define MAINWINDOW_H
 
#include <QMainWindow>
#include "baction.h"
#include "caction.h"
#include "point.h"
#include "serial.h"
#include <QInputDialog>
#include "arrowbutton.h"
 
#include "vector"
using namespace std;
 
class MainWindow : public QMainWindow
{
    Q_OBJECT // Q_OBJECT macro manditory for classes containing signals and slots.
 
public:
    MainWindow(); // Constructor
 
private slots:
    void setBaud(int);
    void setCom(int);
    void setComCustom();
    void open();
    void arrowClicked(QString number);
    void setCalPoint1();
    void setCalPoint2();
    void startDrilling();
    void pauseDrilling();
    void abortDrilling();
    void keyPressEvent(QKeyEvent *key);
    void testSlot(QString message);
    void sendData();
    //void calPoint1Changed(int value);
    //void calPoint2Changed(int value);
    void changeAngle(int);
    void SignalUpdate();
 
private:
    void createMenus(); // Creates the menus
    void createStatusBar(); // Creates the status bar
    void paintEvent( QPaintEvent *event );
    void getDrillCoordinates(QString);
    void calculateGlobalCoordinates();
 
    void mousePressEvent(QMouseEvent * e); // new
 
    void sendGlobalCoordinates();
 
    QMenu *fileMenu;
    QMenu *baudMenu;
    QMenu *comMenu;
    QMenu *helpMenu;
 
    // Declare file menu actions
    QAction *fileExitAction;
    QAction *fileOpenAction;
    // Declare baud menu actions
    BAction *baud110Action;
    BAction *baud600Action;
    BAction *baud2400Action;
    BAction *baud9600Action;
    BAction *baud56000Action;
    BAction *baud115200Action;
    // Declare com menu actions
    CAction *com1Action;
    CAction *com2Action;
    CAction *com3Action;
    CAction *com4Action;
    CAction *com5Action;
    CAction *com6Action;
    CAction *comCustomAction;
    // Declare help menu actions
    QAction *helpContentsAction;
    QAction *helpAboutAction;
 
    QTextEdit *debugOutput;
    QTextEdit *serialOutput;
 
    QVector<int> *xC; // Hole coordinates
    QVector<int> *yC;
 
    //QVector<point> *pointVector;
    vector<point> pointVector;
 
    bool drillFileLoaded;
    bool drillCalibrated;
 
    int desiredX; // desired position of drill from origin in steps
    int desiredY;
    int desiredZ;
 
    QLabel *statusLabel;
    QLabel *drillLoadedLabel;
    QLabel *calibrateStatusLabel;
    QLabel *drillStatusLabel;
    QLabel *boardHeightLabel;
    QLabel *boardWidthLabel;
    QLabel *calibrateLabel;
    QLabel *point1Label;
    QLabel *point2Label;
    QLabel *desiredZLabel;
 
    Thread *myThread;
 
    QRadioButton *stepRadio1;
    QRadioButton *stepRadio2;
    QRadioButton *stepRadio3;
    QRadioButton *stepRadio4;
    QRadioButton *stepRadio5;
    int stepRadio1Value;
    int stepRadio2Value;
    int stepRadio3Value;
    int stepRadio4Value;
    int stepRadio5Value;
 
    int calPoint1Set;
    int calPoint2Set;
 
    QPushButton *point1Button;
    QPushButton *point2Button;
    unsigned int calPoint1;
    unsigned int calPoint2;
 
    QSpinBox *calPoint1SpinBox;
    QSpinBox *calPoint2SpinBox;
 
    QPushButton *startButton;
    QPushButton *pauseButton;
    QPushButton *abortButton;
 
    ArrowButton *upButton;
    ArrowButton *downButton;
    ArrowButton *leftButton;
    ArrowButton *rightButton;
    ArrowButton *zUpButton;
    ArrowButton *zDownButton;
 
    double rotate;
 
    int flipBoard;
 
    int clickedX;
    int clickedY;
    int mouseClicked;
 
 
    double boardWidth;
    double boardHeight;
 
    int globalCoordinatesSent; // 0 = none, 1 = x sent, 2 = y sent.
};
 
#endif // MAINWINDOW_H

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include "baction.h"
#include "caction.h"
#include "point.h"
#include "serial.h"
#include <QInputDialog>
#include "arrowbutton.h"

#include "vector"
using namespace std;

class MainWindow : public QMainWindow
{
    Q_OBJECT // Q_OBJECT macro manditory for classes containing signals and slots.

public:
    MainWindow(); // Constructor

private slots:
    void setBaud(int);
    void setCom(int);
    void setComCustom();
    void open();
    void arrowClicked(QString number);
    void setCalPoint1();
    void setCalPoint2();
    void startDrilling();
    void pauseDrilling();
    void abortDrilling();
    void keyPressEvent(QKeyEvent *key);
    void testSlot(QString message);
    void sendData();
    //void calPoint1Changed(int value);
    //void calPoint2Changed(int value);
    void changeAngle(int);
    void SignalUpdate();

private:
    void createMenus(); // Creates the menus
    void createStatusBar(); // Creates the status bar
    void paintEvent( QPaintEvent *event );
    void getDrillCoordinates(QString);
    void calculateGlobalCoordinates();

    void mousePressEvent(QMouseEvent * e); // new

    void sendGlobalCoordinates();

    QMenu *fileMenu;
    QMenu *baudMenu;
    QMenu *comMenu;
    QMenu *helpMenu;
    
    // Declare file menu actions
    QAction *fileExitAction;
    QAction *fileOpenAction;
    // Declare baud menu actions
    BAction *baud110Action;
    BAction *baud600Action;
    BAction *baud2400Action;
    BAction *baud9600Action;
    BAction *baud56000Action;
    BAction *baud115200Action;
    // Declare com menu actions
    CAction *com1Action;
    CAction *com2Action;
    CAction *com3Action;
    CAction *com4Action;
    CAction *com5Action;
    CAction *com6Action;
    CAction *comCustomAction;
    // Declare help menu actions
    QAction *helpContentsAction;
    QAction *helpAboutAction;

    QTextEdit *debugOutput;
    QTextEdit *serialOutput;

    QVector<int> *xC; // Hole coordinates
    QVector<int> *yC;

    //QVector<point> *pointVector;
    vector<point> pointVector;

    bool drillFileLoaded;
    bool drillCalibrated;

    int desiredX; // desired position of drill from origin in steps
    int desiredY;
    int desiredZ;

    QLabel *statusLabel;
    QLabel *drillLoadedLabel;
    QLabel *calibrateStatusLabel;
    QLabel *drillStatusLabel;
    QLabel *boardHeightLabel;
    QLabel *boardWidthLabel;
    QLabel *calibrateLabel;
    QLabel *point1Label;
    QLabel *point2Label;
    QLabel *desiredZLabel;

    Thread *myThread;

    QRadioButton *stepRadio1;
    QRadioButton *stepRadio2;
    QRadioButton *stepRadio3;
    QRadioButton *stepRadio4;
    QRadioButton *stepRadio5;
    int stepRadio1Value;
    int stepRadio2Value;
    int stepRadio3Value;
    int stepRadio4Value;
    int stepRadio5Value;

    int calPoint1Set;
    int calPoint2Set;

    QPushButton *point1Button;
    QPushButton *point2Button;
    unsigned int calPoint1;
    unsigned int calPoint2;

    QSpinBox *calPoint1SpinBox;
    QSpinBox *calPoint2SpinBox;

    QPushButton *startButton;
    QPushButton *pauseButton;
    QPushButton *abortButton;

    ArrowButton *upButton;
    ArrowButton *downButton;
    ArrowButton *leftButton;
    ArrowButton *rightButton;
    ArrowButton *zUpButton;
    ArrowButton *zDownButton;

    double rotate;

    int flipBoard;

    int clickedX;
    int clickedY;
    int mouseClicked;


    double boardWidth;
    double boardHeight;

    int globalCoordinatesSent; // 0 = none, 1 = x sent, 2 = y sent.
};

#endif // MAINWINDOW_H

To copy to clipboard, switch view to plain text mode 

Use a queue.

I thought about implementing some sort of buffer, but it would rather complicate things and I was just looking for a simple and quick solution..

[Timoteo]

Cleaned this up a little for you.

void MainWindow::sendGlobalCoordinates()
{   
    if(myThread->TXFlag == 0)
    {
 
        if(globalCoordinatesSent == 0) // if x&y not sent, Send the x
        {
	    /*is this actually safe?  what are the assumptions about the validity of these writes?*/
            /*synchronization?*/
            myThread->TXBytes[0] = 56;   // setGlobalPosX
            myThread->TXData = (int)(pointVector[calPoint2].gX * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 1) // If x sent, but y not sent, sent the y
        {
            myThread->TXBytes[0] = 57;   // setGlobalPosY
            myThread->TXData = (int)(pointVector[calPoint2].gY * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 2) // if x&y sent
        {
            startButton->setEnabled(1); // let the drilling begin!
            return; // the recursive loop must be broken.
        }
    }
    sendGlobalCoordinates(); // keep going    
}

void MainWindow::sendGlobalCoordinates()
{   
    if(myThread->TXFlag == 0)
    {
	  
        if(globalCoordinatesSent == 0) // if x&y not sent, Send the x
        {
	    /*is this actually safe?  what are the assumptions about the validity of these writes?*/
            /*synchronization?*/
            myThread->TXBytes[0] = 56;   // setGlobalPosX
            myThread->TXData = (int)(pointVector[calPoint2].gX * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 1) // If x sent, but y not sent, sent the y
        {
            myThread->TXBytes[0] = 57;   // setGlobalPosY
            myThread->TXData = (int)(pointVector[calPoint2].gY * (double)4000);
            myThread->bytesToSend = 4;   // command(1) + data(2) + sign(1)
            myThread->TXFlag = 1;
            globalCoordinatesSent++;
        }
        else if(globalCoordinatesSent == 2) // if x&y sent
        {
            startButton->setEnabled(1); // let the drilling begin!
            return; // the recursive loop must be broken.
        }
    }
    sendGlobalCoordinates(); // keep going    
}

To copy to clipboard, switch view to plain text mode 

[SixDegrees]

Use a queue.

...or a debugger.

[N3wb]

Cleaned this up a little for you.

Thanks, that is a bit clearer.

Unfortunately the program still crashes.

...or a debugger.

I actually tried using the debugger in Qt, but due to my lack of experience I accomplished nothing.

[SixDegrees]

What error messages are produced when the program fails?

[N3wb]

Well, the program window kind of greys out and a Windows dialog box pops up saying "test_project.exe has stopped working." I click "close the program" and in "application output" in Qt Creator it says "... test_project.exe exited with code -1073741571"

#
/*is this actually safe? what are the assumptions about the validity of these writes?*/
#
/*synchronization?*/

There is no confirmation that the data was sent and received correctly. I've never had data lost before so I just assume it transfers OK every time..

[ChrisW67]

Just looking at the few lines at the start:

void MainWindow::sendGlobalCoordinates()
{
  if(myThread->TXFlag == 1)
    {
      return sendGlobalCoordinates(); // try try again
    }

void MainWindow::sendGlobalCoordinates()
{
  if(myThread->TXFlag == 1)
    {
      return sendGlobalCoordinates(); // try try again
    }

To copy to clipboard, switch view to plain text mode 

Assuming that TXFlag is 1 how many times is sendGlobalCoordinates() going to call itself before it returns? For every call there must be a matching return; are you sure you will get that?

Every time you call a recursive function it has to record where to return to and create any new local variables on the stack. If the function calls itself in a tight loop like this the stack grows very quickly and eventually hits a system limit or exhausts system memory. How quickly will that happen? Assuming the TXFlag changes after a 4 characters are sent via the serial port at 9600 baud, that will take about 4 msecs (it may be longer). In that time your tight loop may call itself several thousand times. If the data block is longer then the situation gets worse. If your returns don't match your calls then these resources are permanently tied up.

This problem does not strike me as a classic case for recursion. Have you thought about having the sender thread signal when it is done?

Incidentally, 4 bytes does not seem to be correct: 1 byte command, an int data (4 bytes, but you should use sizeof(int)), and the sign (wherever that is defined).

[SixDegrees]

Code -1073741571 indicates a stack overflow, in line with ChrisW67's post. Sounds like your recursion isn't terminating.

[N3wb]

I still haven't been able to figure this out, so I'm just going to forget trying to use a recursive function. I'm not sure why it's not working, but it's more trouble than it's worth. I'll just use some sort of signal like someone else suggested.

Thanks for all of your help guys!