1

This is how my class looks. My code compiles successfully but when I run it, it crashes and stops.

class Explore{

    public:

        Explore(ros::NodeHandle &nh, tf2_ros::Buffer &buffer);

    private: 

        
        bool is_frontier(size_t mx, size_t my);
        void explore_level_three();
        void go_to_cell(size_t mx, size_t my);
        void publish_markers_array();
        void publish_markers();
        
        
        costmap_2d::Costmap2DROS* global_costmap, *local_costmap;
        costmap_2d::Costmap2D* global_costmap_, *local_costmap_;
        
        ros::NodeHandle nh_;
        ros::Publisher frontier_array_pub, frontier_pub; 

        vector<pair<size_t, size_t> >frontiers;
        
        string global_frame, robot_base_frame;
        unsigned char *global_og;

        size_t size_x, size_y;  
        double init_wx, init_wy; 
        
        int vis[4001][4001], frontier_vis[4001][4001] ; 

};

When I change vis[4001][4001], frontier_vis[4001][4001]; to vis[500][500], frontier_vis[500][500]; the code runs successfully.

This is my main function -

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

    ros::init(argc, argv, "explore_node");

    ros::NodeHandle nh("explore_node"); 

   
    tf2_ros::Buffer buffer(ros::Duration(10));
    tf2_ros::TransformListener tf(buffer);
    
    
    Explore explore_one(nh, buffer);

   
    return 0;
} 

How can I solve this issue?

skpro19
  • 519
  • 7
  • 10

2 Answers2

2

How can I solve this issue?

The memory available for automatic objects is typically very limited. As such, you must allocate all huge objects dynamically. Otherwise your variables will likely consume all of the memory reserved for automatic objects, which results in ... stack overflow.

Having huge member variables would not be a problem if all instances of the class were allocated dynamically, but since that restriction is hard to enforce, it is usually best to simply not have huge member variables.

Your arrays vis and frontier_vis are huge. Simple solution: Use std::vector.

eerorika
  • 232,697
  • 12
  • 197
  • 326
  • @eerokia `since that restriction is hard to enforce` - why do you say that? – skpro19 May 17 '21 at 16:49
  • 1
    @skpro19 Because there is no language mechanism for allowing creation of objects in dynamic memory only. Sure, you could make the constructors private, and provide custom functions that return unique pointers, but writing all those functions can be a lot of boilerplate that would be unnecessary if you simply didn't make the class so huge instead. – eerorika May 17 '21 at 16:53
  • 1
    @skpro19 one way to avoid huge member is to make the members dynamically allocated, preferably with a [library container](https://en.cppreference.com/w/cpp/container) to handle the memory management. [Here is a simple matrix class](https://stackoverflow.com/a/2076668/4581301) that provides a dynamically allocated, self-managed structure that looks to the outside world like a 2D array. – user4581301 May 17 '21 at 17:00
  • 1
    @user4581301 here's another one https://www.boost.org/doc/libs/1_36_0/libs/numeric/ublas/doc/matrix.htm – Aykhan Hagverdili May 17 '21 at 17:02
1

Most probably you are creating an object of Explore on stack which would be giving a Segmentation Fault. You should declare the object dynamically on heap

int main() {
    Explore * obj = new Explore(...);
    delete obj; //since obj is on the heap, 
                //you have to take care of delete. using smart pointers 
                //like std::unique_ptr can efficiently handle the lifetime
    return 0;
}

instead of

int main() {
    Explore obj(...);
    return 0;
}

You are better off with std::vector though.

Gaurav Sehgal
  • 7,422
  • 2
  • 18
  • 34
  • 2
    You're leaking memory. You wouldn't if you used unique_ptr – Aykhan Hagverdili May 17 '21 at 16:37
  • @AyxanHaqverdili what do you mean by that? – skpro19 May 17 '21 at 16:39
  • 2
    @skpro19 When you allocate some memory with `new`, you need to deallocate it with `delete`. But if you use `std::unique_ptr`, this happens automatically. – Aykhan Hagverdili May 17 '21 at 16:40
  • @Gaurav Sehgal are there any performance issues with creating on `stack` vs `heap`? If no, why wouldn't one always declare objects on heap so as to avoid (aforementioned) errors. – skpro19 May 17 '21 at 16:42
  • 1
    @skpro19 there are often performance penalties when you allocate storage from the heap. The system has to find free storage of sufficient size. To get memory from the stack all that usually happens is a pointer is moved. You may also have performance penalties when using dynamically allocated objects as they can be scattered throughout memory making it harder to cache them. This should not be a problem with your class due to its large size. – user4581301 May 17 '21 at 16:45
  • @AyxanHaqverdili why should I care about using `delete obj`? – skpro19 May 17 '21 at 16:47
  • 3
    @skpro19 2 main reasons are: You will leak memory (and possibly run out of it) and destrucotrs of leaked objects won't be run. – Aykhan Hagverdili May 17 '21 at 16:52
  • 1
    If the program allocates memory, uses it, and exits without putting the memory back, the underlying system will take care of it on any system I've used in the past few decades. That said, the destructors still won't be run, and that can have significant negative impacts. – user4581301 May 17 '21 at 16:54
  • 2
    @user4581301 It's possible to run out of memory while the program is running. Especially considering OP allocates huge chunks. – Aykhan Hagverdili May 17 '21 at 16:55
  • @user4581301 In addition to what Ayxan said, consider writing a daemon process. Getting the memory back when the program exits is hardly a consolation when the program is "never" supposed to exit. – eerorika May 17 '21 at 16:57