Segmentation Fault when saving ICP map #479
Comments
|
Can you post a code snippet of what you are actually doing? Also if you are feeling particularly adventurous try it against mrpt-2.0. The pointer and serialization code was overhauled and I want to see what kind of exception you get. |
|
Hi, I agree with @jolting : you would need to share more details on your application to help finding out what's wrong... |
|
Hi @jolting it is not an exception but a segmentation fault, I've tried 2 variations of the same thing: mrpt::maps::CSimpleMap out_map;
__builder__.getCurrentlyBuiltMap(out_map);
out_map.saveToFile("simplemap.gz");I've also tried setting the current map file __builder__.initialize(__map__);where @jlblancoc the maps are generated, especially the multimetric maps, its the simple map that fails to save to disk, when using Please bear in mind I've put the code in a ROS node, but the gdb output is consistent: |
|
Are you using the package from the ppa? Please install the -dbg package and
rerun your test with gdb. That should give us line numbers.
…On Thu, Mar 16, 2017, 4:59 AM Alex Giokas ***@***.***> wrote:
Hi @jolting <https://github.com/jolting> it is not an exception but a
segmentation fault, I've tried 2 variations of the same thing:
mrpt::maps::CSimpleMap out_map;
__builder__.getCurrentlyBuiltMap(out_map);
out_map.saveToFile("simplemap.gz");
I've also tried setting the current map file setCurrentMapFile when
starting ICP map builder,
and then saving it:
__builder__.initialize(__map__);
where __map__ and __builder__ are private members, and then later saving
it as before (this actually works).
@jlblancoc <https://github.com/jlblancoc> the maps are generated,
especially the multimetric maps, its the simple map that fails to save to
disk, when using getCurrentlyBuiltMap.
Please bear in mind I've put the code in a ROS node, but the gdb output is
consistent:
Thread 12 "mario_node" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffbaffd700 (LWP 9266)]
0x00007ffff4363f93 in __dynamic_cast () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0 0x00007ffff4363f93 in __dynamic_cast () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1 0x00007ffff5ea06de in mrpt::obs::CSensoryFrame::writeToStream(mrpt::utils::CStream&, int*) const () from /usr/lib/x86_64-linux-gnu/libmrpt-obs.so.1.5
#2 0x00007ffff6ca513e in mrpt::utils::CStream::WriteObject(mrpt::utils::CSerializable const*) () from /usr/lib/x86_64-linux-gnu/libmrpt-base.so.1.5
#3 0x00007ffff6ca5629 in mrpt::utils::CStream::operator<<(mrpt::utils::CSerializable const&) () from /usr/lib/x86_64-linux-gnu/libmrpt-base.so.1.5
#4 0x00007ffff5e2837a in mrpt::maps::CSimpleMap::writeToStream(mrpt::utils::CStream&, int*) const () from /usr/lib/x86_64-linux-gnu/libmrpt-obs.so.1.5
#5 0x00007ffff6ca513e in mrpt::utils::CStream::WriteObject(mrpt::utils::CSerializable const*) () from /usr/lib/x86_64-linux-gnu/libmrpt-base.so.1.5
#6 0x00007ffff6ca5629 in mrpt::utils::CStream::operator<<(mrpt::utils::CSerializable const&) () from /usr/lib/x86_64-linux-gnu/libmrpt-base.so.1.5
#7 0x00007ffff5e2c9ff in mrpt::maps::CSimpleMap::saveToFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const () from /usr/lib/x86_64-linux-gnu/libmrpt-obs.so.1.5
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#479 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAOu9ccfx5bkzvEFd2CmBCbyD9htga98ks5rmSQGgaJpZM4MeIIF>
.
|
|
@jolting Hi, yes I'm using PPA
However there is no |
|
Try my ppa if you are using 16.04. https://launchpad.net/~jolting/+archive/ubuntu/mrpt-daily Note: I can't guarantee stability of my PPA. Only use it for debugging. |
|
HI @jolting I've tried your ppa, but it still doesn't install When I try with The following packages have unmet dependencies.
libmrpt-dbg : Depends: libmrpt-dev (= 1:1.5.0~snapshot20160512-1218-git-ebcee4ef-xenial-1~ppa1~xenial) but 1:1.5.0~snapshot20170307-0309-git-61268233-xenial-1~ppa1~xenial is to be installed
E: Unable to correct problems, you have held broken packages. |
|
Did you installed this one? |
|
yes I did, I even removed the cached cmake build stuff just to be sure... |
|
Ah! Let's @jolting give his opinion here, but I think we had a misunderstood, sorry: Then, gdb should be able, so goes on the theory, to find the symbols and print a more informative stack trace. |
|
I'm a bit confused, should I build it from source with debug symbols? AFAIK the ppa packages do not include debug symbols. |
|
Add don't use debug libs macro to your cmake file. |
|
Hi, the only thing I'm adding is the cmake flag |
|
Did you download a newer build? The latest one was built 7 hours ago. The -dbg issue should have been fixed 3 days ago with this commit. |
|
@jolting I'm away this week so can't access the robot; I'll try rebuilding from github |
|
@jolting Hi I'm back this week. I'm trying to build from github ( Should I try the one from your repository instead? |
|
Yes, you can try the repo on 16.04. To address your issue try installing |
|
Sorry for taking so long to reply, here is what happens: |
|
https://github.com/MRPT/mrpt/blob/master/libs/base/src/utils/CStream.cpp#L193 Go up to function 6 and print o. |
|
The observation was a |
|
In that context the type of Is it possible that you are assigning a raw pointer to a smart pointer twice? |
|
Hi, I am populating observation from a ros laserscan message. I allocate an object, and then assign a pointer using that object, and finally insert the observation. ICP works fine, I get no segfaults, crashes or leaks, the only exception is when I use that particular method. The (overall and simplified) version of this is: void sick_561::populate_lazer(const sensor_msgs::LaserScan::ConstPtr & data)
{
auto obs = mrpt::obs::CObservation2DRangeScan();
obs.loadFromVectors(data->ranges.size(),
data->ranges.data(),
(char*)data->intensities.data());
auto ptr = mrpt::obs::CObservation2DRangeScanPtr();
ptr.setFromPointerDoNotFreeAtDtor(&obs);
builder__.processObservation(ptr);
}The ICP class wrapper then goes on to write the map, at which point I get the segmentation fault: builder__.saveCurrentMapToFile("icpmap", true);The other method builder__->getcurrentlyBuiltMetricMap()->m_gridMaps[0]->saveAsBitmapFile("gridmap.png");
builder__->getcurrentlyBuiltMetricMap()->m_pointsMaps[0]->save3D_to_text_file("pointsmap.txt");I went to I did: Does that help? |
|
You're not at the right stack level. Assuming that old stack trace is somewhat relevant... The useful data is here: |
|
Hi Alex Your problem is that you're populating the smart pointer with a pointer to a local variable, which is allocated the stack! This is one of the biggest DONT with smart pointers ;-) Just create the smart pointer FIRST, with something like: auto obs = CObservation2DRangeScan::Create(); Cheers |
|
@jlblancoc this should not be a problem, unless the builder requires a copy, in which case it should make a deep copy. From what I understand the method expects a heap-allocated object which the builder will deallocate? |
|
@alexge233 The purpose of the shared pointer is to prevent the overhead of the deep copy. It just stores a copy of the the smart pointer, so it's best just to let the smart pointer manage the lifetime of the object. Basically the object must exist after This stack allocates the object: This makes the shared pointer from a stack allocated object. This is bad Try this: I find that smart pointers are a bit burdensome to learn. I've attempted to teach them a several times to classmates with varying levels of success. I had an idea for mrpt-2.0 about adding a version that does not depend on pointers. Instead it would depend on c++11 move semantics/copy semantics. A move would be zero copy and a copy would then have an overhead of a deep copy. Personally, I find these concepts simpler and easier to express. Unfortunately, we aren't there yet in the 2.0 branch. Also, this idea might be trading one problem for another because many developers haven't quite grasped move semantics yet. |
|
@jolting thanks for the help! I'm not criticising guys, we appreciate a lot the effort you've put into this library. If however you intend to use smart pointers, may I suggest you go down the STL route instead? For example, if you use an Following the documentation from doxygen for 1.5.0 its wasn't clear if those methods are expecting an object (copyable), a heap-allocated object or whatnot; the fact you that you have Even worse, I was using a scoped object on purpose, assuming you made a copy of it and ICP was working fine, it wasn't until I called the method Again, many thanks for the help! |
|
I'm glad it's solved and clear now.
Mrpt was started in ~2004... that's why we couldn't use std real smart
pointers, introduced in C++11 ;-)
That's fixed in the 2.0 branch, though!
Cheers
|
|
@alexge233 Feel free to help test mrpt-2.0. |
|
@jolting is it in your ppa or should I build it from source? We're mostly interested in ICP and visual slam. Do you have a gitter room for mrpt? |
I'm using mrpt 1.5 from an Ubuntu PPA.
The ICP builder exposes some methods which AFAIK allow the
CSimpleMapto be saved, however they all SEGFAULT when I try:saveCurrentMapToFilesegfaults right awaysetCurrentMapFilewhen the builder is destroyed, it tries to save the map filegetCurrentlyBuiltMapdoes not fill the map, which when I try to save segfaults alsoAm I using this wrongly, or is this a bug?
Running under
gdband backtracing pinpoints tomrpt::obs::CSensoryFrame::writeToStreamwhich in turn hints towards adynamic_cast.The text was updated successfully, but these errors were encountered: