Yarp: gravityCompensator crashes in the invocation of yarp::sig::Vector::operator=(&&)

Created on 11 Dec 2020  路  9Comments  路  Source: robotology/yarp

This issue is for discussing the problem we discovered recently on gravityCompensator (https://github.com/robotology/icub-main/pull/702).

In few words it crashed sistematically at this line:

https://github.com/robotology/icub-main/blob/78be1eff4dfda699690f9a2b4310cee6d7d39bcf/src/modules/gravityCompensator/gravityThread.cpp#L748

On that yarp::sig::Vector (exec_torques_RA), as the other many Vectors in gravityCompensator, a resize(7) and zero() was invoked in the threadInit().

We verified that all those vectors involved in that operation had correct sizes.

We tried then to put the result of that variable in a temporary Vector, and then copy that Vector in "our" exec_torques_RA and it worked smoothly. Since the temporary Vector had a name, then the copy assignment operator was invoked instead of the move one.

Initializing the Vectors with the correct size in the gravityThread constructor(see https://github.com/robotology/icub-main/pull/702/commits/00f904632012eed4b222880f42ea035f47eadd2c) seems to fix the problem.

Under the hood of yarp::sig::Vector there is a std::vector, since a while (https://github.com/robotology/yarp/pull/2195) and it is the first time I experience a problem like this

The implementations of copy/move assignment operators seems pretty straightforward and I don't see any criticity:

It seems strange to me that operator=(&&) di std::vector is bugged.

It seems that doing

std::vector<double> v;
v.resize(7);
std::fill(v.begin(), v.end(), 0.0);

is different from

std::vector<double> v(7, 0.0);

Maybe it is due to the allocation strategy is implementation-dependent, under some OS(e.g. Linux) should allocate the memory by power of 2 elements(https://stackoverflow.com/questions/11560904/vector-memory-allocation-strategy)

This issue is not mainly focused on gravityCompensator (that is in another repo) but on the possibility that we could have a hidden issue in yarp::sig::Vector, and since it is widely used, it is quite dangerous.

The strange thing is that before everything was working fine and nothing changed on yarp::sig::Vector and gravityCompensator side.

cc @traversaro , @pattacini . @triccyx , @davidetome , @drdanz

YARP v3.4.1 Library - YARP_sig Bug Fixed

All 9 comments

The copy/move ctor/operator are no longer doing anything that it isn't done by the default copy/move ctor/operator. I think we can just remove them (rule of zero).
Can you try and see if it works after removing them?

The copy/move ctor/operator are no longer doing anything that it isn't done by the default copy/move ctor/operator. I think we can just remove them (rule of zero).
Can you try and see if it works after removing them?

Good point! I can try it!

@Nicogene did you have any chance to try this?

I didn't, but I think that it can be a PR that can be opened in any case

PR opened, see #2471

@Nicogene #2471 was merged, let me know if it works now...

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

Sorry for the late reply, actually after this patch the crashes involves also wholeBodyDynamics but I suspect that it is not a problem of the yarp::sig::Vector

As I suspected this issue was not yarp::sig::Vector fault: https://github.com/robotology/icub-main/pull/729

Closing.

Was this page helpful?
0 / 5 - 0 ratings