Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fromCvMat in camcalib produce segFault #1979

Closed
gonzalezJohnas opened this issue Mar 18, 2019 · 2 comments
Closed

fromCvMat in camcalib produce segFault #1979

gonzalezJohnas opened this issue Mar 18, 2019 · 2 comments
Assignees
Labels
Component: Library - YARP_sig Fixed in: YARP v3.2.0 Issue Type: Bug Involves some intervention from a system administrator

Comments

@gonzalezJohnas
Copy link

gonzalezJohnas commented Mar 18, 2019

This bug has been found and solve with the help of Andrea Ruzzenenti (@aerydna)

Describe the bug
When running camCalib from icub-main and more specifically in the PinholeCalibTool::apply function produce a null image after fromCvMat<PixelRgb> resulting in a segfault.

We found that the issue is in reality in libYARP_sig/src/Image.cpp for the function Image& Image::operator=(Image&& other).

To Reproduce
Connect camCalib module to yarpdev (grabberDual)

Expected behavior
The image returned from fromCvMat<PixelRgb> after applying the calibration with remap is not valid so accessing is pixel produce a segfault.

Configuration :

  • OS: Ubuntu 18.04
  • yarp version: 3.0 branch devel
  • compiler: gcc, g++ version 7.3.0

Temporary fix
The actual bug happen in the libYARP_sig/src/Image.cpp :

Image& Image::operator=(Image&& other) {
    Image moved(std::move(other));
    std::swap(moved.implementation, implementation);
    synchronize();
    return *this;
}

When performing the swap if moved.implementation is not set to nullptr, the Image destructor is called on moved destroying also the implementation of the current object (this).

A way to fix it is to explicitly set moved.implementation to nullptr before the synchronize or before the swap.

For more details you can contact me or Andrea Ruzzenenti (@aerydna)

@drdanz
Copy link
Member

drdanz commented Mar 19, 2019

Thanks for reporting this.
It's fixed in devel now.

@Nicogene
Copy link
Member

The move operators were correct, the commit that add nullptr has been reverted since it introduced memory leak.

The bug was in camCalib, it has been fixed by robotology/icub-main#572

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Library - YARP_sig Fixed in: YARP v3.2.0 Issue Type: Bug Involves some intervention from a system administrator
Projects
None yet
Development

No branches or pull requests

4 participants