CODEWITHSUNDEEP
c++qtboostpoint-cloud-libraryheap-corruption
Fred
Fred

Reputation: 3362

Error when a shared pointer goes out of scope in a loop (Heap corruption?)

I couldn't find the problem by stepping through the code line by line.

I managed to extract a minimal example from the codebase and it all boils down to the following lines. What the code does is that it reads a 3D point cloud from an object, wraps it into a shared pointer and sends it away with QT's signal engine. The two lines in the middle are causing the error:

for(vector<Package>::iterator resit = results.begin(); resit != results.end(); resit++) {
    // [..] Code ommitted
    pcl::PointCloud<pcl::PointXYZ> c = queryObject.getCloud();


    // The Ptr typedef resolves to boost::shared_ptr<PointCloud<PointT>>
    // (1) Does not work:   
    pcl::PointCloud<pcl::PointXYZ>::Ptr cloud_ptr(&c);
    // (2) Works:
    //pcl::PointCloud<pcl::PointXYZ>::Ptr cloud_ptr(new pcl::PointCloud<pcl::PointXYZ>);


    emit this->cluster_added(cloud_ptr);
}
// The error always happens AFTER the FIRST iteration

The code works when I comment in (2) (comment out (1) of course..). In both versions cloud_ptr is a shared pointer carrying cloud - except for the fact that the first time it's a populated cloud while it's not in the second version.

Edit: Since you guys pointed it out - I see how messed up the current version is. That's the result of pointless tryouts.. Initially the getCloud() method returned a pointer to the cloud. But that version didn't work either. This is the original version of the code:

for(vector<Package>::iterator resit = results.begin(); resit != results.end(); resit++) {
    // [..] Code ommitted

    pcl::PointCloud<pcl::PointXYZ>::Ptr cloud_ptr(queryObject.getCloud());


    emit this->cluster_added(cloud_ptr);
}

Edit #2: Solution

Turns out I had a huge misunderstanding about boost pointers. The correct approach here is to create the pointer along with the point cloud and then pass the thing to the object:

// Object creation
pcl::PointCloud<pcl::PointXYZ>::Ptr cloud (new pcl::PointCloud<pcl::PointXYZ>);
// Do some loading..
[..]

// Use the cloud in the for loop
emit this->cluster_added(queryObject.getCloud());

Heaperror2 Heaperror1

Upvotes: 1

Views: 1515

Answers (1)

TartanLlama
TartanLlama

Reputation: 65580

Your issue is that are creating a stack-allocated variable here:

pcl::PointCloud<pcl::PointXYZ> c = queryObject.getCloud();

This variable will be destroyed when it goes out of scope, so wrapping it up into a std::shared_ptr doesn't make sense.

How you fix the issue depends on the semantics of your PointCloud class with regards to copying, but I'd guess that using this line would fix the issue:

pcl::PointCloud<pcl::PointXYZ>::Ptr cloud_ptr(new pcl::PointCloud<pcl::PointXYZ>(c));

or, if you don't expect that pointer type to change:

auto cloud_ptr = {make_shared<pcl::PointCloud<pcl::PointXYZ>>(c)};

Upvotes: 8

Related Questions