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

More nullptr checking on Node classes #364

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

chapulina
Copy link
Contributor

🦟 Bug fix

Summary

This is motivated by a crash I had on ign-gazebo with this backtrace:

full backtrace

/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v58Ogr
eNode11DetachChildESt10shared_ptrINS1_4NodeEE+0x74) [0x7efb8e273984] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/ogre/src/OgreNo
de.cc:161
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v58Bas
eNodeINS1_10OgreObjectEE11RemoveChildESt10shared_ptrINS1_4NodeEE+0xcb) [0x7efb8e20a91b] /home/chapulina/dev_bionic/ws_edifice/src/ign-render
ing/include/ignition/rendering/base/BaseNode.hh:239
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v58Bas
eNodeINS1_10OgreObjectEE12RemoveParentEv+0x10e) [0x7efb8e20c2fe] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/include/ignition/re
ndering/base/BaseNode.hh:213
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v58Ogr
eNode7DestroyEv+0x9) [0x7efb8e272fe9] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/ogre/src/OgreNode.cc:60
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v510Ba
seVisualINS1_8OgreNodeEE7DestroyEv+0xa9) [0x7efb8e20b7c9] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/include/ignition/rendering
/base/BaseVisual.hh:318
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v59Bas
eStoreINS1_6VisualENS1_10OgreVisualEE11DestroyImplESt17_Rb_tree_iteratorISt4pairIKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt10sh
ared_ptrIS4_EEE+0x39) [0x7efb8e263dd9] /usr/include/c++/8/bits/shared_ptr_base.h:728
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v59Bas
eStoreINS1_6VisualENS1_10OgreVisualEE10DestroyAllEv+0x32) [0x7efb8e260ad2] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/include/i
gnition/rendering/base/BaseStorage.hh:730
/home/chapulina/dev_bionic/ws_edifice/install/lib/libignition-rendering5.so.5(_ZN8ignition9rendering2v518BaseCompositeStoreINS1_4NodeEE10Des
troyAllEv+0x2e) [0x7efbe02a5b0e] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/include/ignition/rendering/base/BaseStorage.hh:1216
/home/chapulina/dev_bionic/ws_edifice/install/lib/libignition-rendering5.so.5(_ZN8ignition9rendering2v59BaseScene5ClearEv+0x14) [0x7efbe029b
2e4] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/src/base/BaseScene.cc:1241
/home/chapulina/dev_bionic/ws_edifice/install/lib/libignition-rendering5.so.5(_ZN8ignition9rendering2v59BaseScene7DestroyEv+0xd) [0x7efbe029
b31d] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/src/base/BaseScene.cc:1250
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v59Ogr
eScene7DestroyEv+0x9) [0x7efb8e287ce9] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/ogre/src/OgreScene.cc:296
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v59Bas
eStoreINS1_5SceneENS1_9OgreSceneEE11DestroyImplESt17_Rb_tree_iteratorISt4pairIKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt10share
d_ptrIS4_EEE+0x3c) [0x7efb8e26327c] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/include/ignition/rendering/base/BaseStorage.hh:9
56
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-rendering-5/engine-plugins/libignition-rendering-ogre.so(_ZN8ignition9rendering2v59Bas
eStoreINS1_5SceneENS1_9OgreSceneEE7DestroyESt10shared_ptrIS3_E+0x70) [0x7efb8e2647a0] /home/chapulina/dev_bionic/ws_edifice/src/ign-renderin
g/include/ignition/rendering/base/BaseStorage.hh:699
/home/chapulina/dev_bionic/ws_edifice/install/lib/libignition-rendering5.so.5(_ZN8ignition9rendering2v516BaseRenderEngine12DestroySceneESt10
shared_ptrINS1_5SceneEE+0x66) [0x7efbe029a3f6] /home/chapulina/dev_bionic/ws_edifice/src/ign-rendering/src/base/BaseRenderEngine.cc:163
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-gazebo-5/plugins/gui/libGzScene3D.so(_ZN8ignition6gazebo2v511IgnRenderer7DestroyEv+0x2
ac) [0x7efc001523cc] /home/chapulina/dev_bionic/ws_edifice/src/ign-gazebo/src/gui/plugins/scene3d/Scene3D.cc:1905
/home/chapulina/dev_bionic/ws_edifice/install/lib/ign-gazebo-5/plugins/gui/libGzScene3D.so(_ZN8ignition6gazebo2v512RenderThread8ShutDownEv+0
x25) [0x7efc00152475] /home/chapulina/dev_bionic/ws_edifice/src/ign-gazebo/src/gui/plugins/scene3d/Scene3D.cc:2311

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@chapulina chapulina requested a review from iche033 as a code owner July 16, 2021 18:28
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Jul 16, 2021
@codecov
Copy link

codecov bot commented Jul 16, 2021

Codecov Report

Merging #364 (83c1590) into ign-rendering5 (87b30ad) will decrease coverage by 0.08%.
The diff coverage is 40.90%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering5     #364      +/-   ##
==================================================
- Coverage           57.80%   57.71%   -0.09%     
==================================================
  Files                 161      161              
  Lines               15851    15901      +50     
==================================================
+ Hits                 9163     9178      +15     
- Misses               6688     6723      +35     
Impacted Files Coverage Δ
ogre/src/OgreNode.cc 45.19% <29.41%> (-7.38%) ⬇️
ogre2/src/Ogre2Node.cc 80.18% <53.12%> (-11.78%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87b30ad...83c1590. Read the comment docs.

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@chapulina chapulina enabled auto-merge (squash) July 16, 2021 19:04
@chapulina chapulina merged commit 44a1948 into ign-rendering5 Jul 16, 2021
@chapulina chapulina deleted the chapulina/5/node_ptrs branch July 16, 2021 19:10
adlarkin pushed a commit that referenced this pull request Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants