Search code examples
c++classfunctor

Why does this program crash or hang?


The following code crashes or hangs until memory exhaustion:

#include <iostream>
#include <functional>
using namespace std::placeholders;

struct Goal {float x; float y;};

class Boxbot 
{
public:
    Boxbot(Goal cgoals[]=0) 
    {
        for (int i=0;i<3;i++) { cgoals_[i]=cgoals[i]; }
        for (int i=0;i<3;i++) { callbacks(i); }
        
    }
private:
    //Declare member function
    bool check_goal(Goal gl) 
    {
        if (std::sqrt(std::pow(gl.x,2)+std::pow(gl.y,2))<2) {return true;}
        else {return false;}
    }
    //Creating a functor : function object
    struct Callback 
    {
        void operator() (int i) const {
            Boxbot b;
            b.bgoals[i]=b.check_goal(b.cgoals_[i]);
            std::printf("%s is %i\n",b.topicns[i].data(),b.bgoals[i]);
        }
    } callbacks;
    
    //Declare member variables
    bool bgoals[3]={false,false,false};
    Goal cgoals_[3];
    std::string topicns[3]={"robot_1","robot_2","robot_3"};
};

int main() 
{
    Goal cgoals[3] = {{0.3,0.6},{0.9,1.2},{1.5,1.8}};
    Boxbot boxbot(cgoals);
    return 0;
}

The result should be as follows, but it freezes.

robot_1 is 1
robot_2 is 1
robot_3 is 0

All I want to do here is this part:for (int i=0;i<3;i++) { callbacks(i); }. In other words, I would like to create a function object in private and access it in public to pass the arguments from main.

The check_goal() & Callback itself are working fine when I put them in public, so i think the issue is the functor callbacks does'nt work in public well... but I do not why.

I tested on C++ shell version 14, 17, 20.


Solution

  • You have two problems, one leading to the other.

    The first problem is that you have an infinite recursion of Boxbot creations.

    In the main function you create one Boxbot object.

    In the Boxbot class you have

    struct Callback 
    {
        void operator() (int i) const {
            Boxbot b;   // Here you create another Boxbot object
            b.bgoals[i]=b.check_goal(b.cgoals_[i]);
            std::printf("%s is %i\n",b.topicns[i].data(),b.bgoals[i]);
        }
    } callbacks;
    

    In the operator() function you create another Boxbot object. And this operator is called in the Boxbot constructor. So every Boxbot object creation will lead to another Boxbot` object creation. Forever, or until you get a stack overflow.


    The second problem comes because the constructor defaults the arguments to be a null pointer (Goal cgoals[]=0 as an argument is really Goal* cgoals=0) your recursive object creations will have a null-pointer access when you copy the Goal objects. That leads to undefined behavior and a likely crash.


    And I'm not really sure why you have Callback and the callbacks object to begin with. Your constructor could just be:

    Boxbot(Goal* cgoals) 
    {
        if (cgoals == nullptr)
        {
            return;
        }
    
        for (size_t i = 0; i < 3; ++i)
        {
            cgoals_[i] = cgoals[i];
            bgoals[i] = check_goal(cgoals_[i]);
        }
    }
    

    No recursion, no null-pointer access, and only one loop needed.