#1859 Declined
Repository
Branch
issue_1702
Repository
Branch
default
Author
  1. John Hsu
Reviewers
Description
  • change name of visuals as a hackish temporary fix. The real fix is to figure out how to properly delete ogre scene nodes.
  • add a regression test
  • Issues #1723: DART link destruction segfault new

Comments (43)

  1. Louise Poubel

    These visuals had empty destructors which weren't cleaning up everything properly. I removed them, so the Visual base class destructor is used instead and your test passes. I also removed your unique name changes. See commit 3ceaa8b, feel free to merge if you think it's ok.

    One thing I noticed though was that the test fails for Simbody right on the first ServerFixture::SpawnModel, so I left simbody out for now...

      1. John Hsu author

        hm... I have not been able to reproduce failure with simbody. What kind of error did you actually see?

        1. Louise Poubel

          Actually I just tried running gazebo with simbody and it crashes whenever I insert a box, so the problem is not with the test per se. It could be my installation... I'm ok with doing this test for simbody and investigating my failure separately.

          Running the test several times, it always fails on the first SpawnModel. Most times with this message:

          *** Error in `./test/regression/REGRESSION_1702_remove_model_scene_nodes': malloc(): memory corruption (fast): 0x00007fd31c3c3131 ***
          Aborted (core dumped)
          

          and a few times with this one:

          REGRESSION_1702_remove_model_scene_nodes: /usr/include/simbody/simbody/internal/ContactSurface.h:39: SimTK::ContactCliqueId::ContactCliqueId(int): Assertion `i>=0 || i==SimTK::InvalidIndex' failed.
          Aborted (core dumped)
          
    1. Steve Peters

      Clarification: I think this indicates a problem, since the test should fail without the fix

    2. John Hsu author

      hm, the failure happens on gzclient, not on gzserver. By changing the world from empty.world to camera.world, the test fails.

      unfortunately, @chapulina s patch does not fix the error. My original hack does temporarily get around it though (see issue_1702_john branch)

        1. John Hsu author

          note the failure mode is exception thrown by server->Run(), so not a typical gtest failure.

          1. John Hsu author

            So the test fails in default and in branch issue_1702, (but not in issue_1702_john with a hack). I am going to look some more at @chapulina 's solution and see if we can get issue_1702 branch to pass the test.

            1. John Hsu author

              OK, test should fail now

              331e9f3

              but not fail due to gtest failure, fail due to

              [==========] Running 1 test from 1 test case.
              [----------] Global test environment set-up.
              [----------] 1 test from PhysicsEngines/Issue1702Test
              [ RUN      ] PhysicsEngines/Issue1702Test.SpawnDeleteSpawnAgain/0
              [Msg] Waiting for master.
              [Msg] Connected to gazebo master @ http://127.0.0.1:11345
              [Msg] Publicized address: 192.168.11.15
              [Dbg] [ServerFixture.cc:169] ServerFixture load in 1.1 seconds, timeout after 600 seconds
              /home/hsu/projects/gazebo_quick/gazebo/test/ServerFixture.cc:242: Failure
              Expected: this->server->Run() doesn't throw an exception.
                Actual: it throws.
              [Dbg] [ServerFixture.cc:118] ServerFixture::Unload
              [  FAILED  ] PhysicsEngines/Issue1702Test.SpawnDeleteSpawnAgain/0, where GetParam() = "ode" (2063 ms)
              [----------] 1 test from PhysicsEngines/Issue1702Test (2063 ms total)
              
              [----------] Global test environment tear-down
              [==========] 1 test from 1 test case ran. (2063 ms total)
              [  PASSED  ] 0 tests.
              [  FAILED  ] 1 test, listed below:
              [  FAILED  ] PhysicsEngines/Issue1702Test.SpawnDeleteSpawnAgain/0, where GetParam() = "ode"
              
               1 FAILED TEST
              terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::lock_error> >'
                what():  boost: mutex lock failed in pthread_mutex_lock: Invalid argument
              Aborted (core dumped)
              

              Note that the test passes with my hack in issue_1702_john branch.

                1. John Hsu author

                  I am debugging the proper ogre scene node removal. My workaround is not a real fix, it will leave behind things in memory.

                      1. John Hsu author

                        If by late tonight a proper solution isn't found, I'll update the PR to my hack so at least gazebo doesn't segfault.

                        1. Louise Poubel

                          I've looked at it and confirmed that it works with your hack. Very strange... I was trying to figure out why the COM and Inertia visuals behave differently from say the LinkFrame visual... LinkFrame was implemented later, so it might have some extra detail we're missing.

  2. Steve Peters

    some code_check errors https://drone.io/bitbucket.org/osrf/gazebo/8781

    test/regression/1702_remove_model_scene_nodes.cc:79:  Missing space before ( in while(  [whitespace/parens] [5]
    test/regression/1702_remove_model_scene_nodes.cc:85:  Consider using EXPECT_LT instead of EXPECT_TRUE(a < b)  [readability/check] [2]
    test/regression/1702_remove_model_scene_nodes.cc:93:  Missing space before ( in while(  [whitespace/parens] [5]
    Total errors found: 3
    
  3. Ian Chen

    I made some changes to clean up the ogre scene nodes and entities in a visual in issue_1702_ian branch. Test passes. The fix should also be backwards compatible.

      1. John Hsu author

        maybe we should recreate the current PR+patch for gazebo6, and create a new PR based on Nate's branch for default?

  4. Louise Poubel

    cppcheck

    gazebo/rendering/COMVisual.hh:64:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
    gazebo/rendering/Visual.cc:172:  Blank line at the start of a code block.  Is this needed?  [whitespace/blank_line] [2]
    
  5. Louise Poubel

    Another way to reproduce the original issue is to:

    1. Insert a box

    2. Copy the box and paste it

    3. Delete the copy

    4. Paste again - crash, because it would have the same name as the deleted one

    On this branch, sometimes (most times?) the procedure above works. But at times:

    • The second pasted box disappears after a while from the 3D scene, but the name remains on the list. Clicking on the name makes it disappear. No crash, no error msgs.

    • I've also had a crash once but can't reproduce it anymore and have no backtrace.