Retry fix for unsafe initialization in graph classes #612
Conversation
* Avoid constructor/destructor fiascos in graph class * Deprecations and migration guide. Signed-off-by: Carlos Agüero <[email protected]> Signed-off-by: Carlos Agüero <[email protected]> Co-authored-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
|
using the |
include/gz/math/graph/Edge.hh
Outdated
| template<typename E, typename EdgeType> | ||
| EdgeType &NullEdge() | ||
| { | ||
| static EdgeType e( |
There was a problem hiding this comment.
This is still problematic depending the given types E, and EdgeType because the destructor eventually runs when the program exits. According to the Google style guide, it's best to allocate memory from the heap and never delete it, which will avoid calling the destructor. gz::utils::NeverDestroyed does exactly that, so I recommend we switch to that.
There was a problem hiding this comment.
trying NeverDestroyed in 3fa9854
There was a problem hiding this comment.
rerunning gz-sim CI https://build.osrfoundation.org/job/gz_sim-ci-pr_any-homebrew-amd64/693/
Signed-off-by: Steve Peters <[email protected]>
🦟 Bug fix
Fixes #269.
Summary
The is a second attempt at fixing the potentially unsafe initialization issues in the
graph::Edgeandgraph::Vertexclasses after the first attempt in #606 was reverted in #609 due to test failures that it caused in gz-sim (documented in osrf/buildfarm-tools#67 (comment)). This pull request uses a static local variable instead of dynamic memory allocation and doesn't cause gz-sim test failures in my testing.Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.