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

Memory leak in ImageDisplay with non-RGB data #341

Closed
peci1 opened this issue Dec 23, 2021 · 12 comments
Closed

Memory leak in ImageDisplay with non-RGB data #341

peci1 opened this issue Dec 23, 2021 · 12 comments
Labels
bug Something isn't working

Comments

@peci1
Copy link
Contributor

peci1 commented Dec 23, 2021

Environment

  • OS Version: Ubuntu 18.04
  • Source or binary build? Source (4.3.0 or 4.4.0)

Description

  • Expected behavior: No memory leaks :)
  • Actual behavior: Adding a display with a depth or thermal camera leaks GUI memory.

Steps to reproduce

gui_leak.sdf
<?xml version="1.0" ?>
<sdf version="1.6">
  <world name="gui_leak">
    <physics name="1ms" type="ignored">
      <max_step_size>0.001</max_step_size>
      <real_time_factor>1.0</real_time_factor>
    </physics>
    <plugin
      filename="ignition-gazebo-sensors-system"
      name="ignition::gazebo::systems::Sensors">
      <render_engine>ogre2</render_engine>
    </plugin>
    <plugin
      filename="ignition-gazebo-scene-broadcaster-system"
      name="ignition::gazebo::systems::SceneBroadcaster">
    </plugin>

    <light type="directional" name="sun">
      <cast_shadows>true</cast_shadows>
      <pose>0 0 10 0 0 0</pose>
      <diffuse>0.8 0.8 0.8 1</diffuse>
      <specular>0.2 0.2 0.2 1</specular>
      <attenuation>
        <range>1000</range>
        <constant>0.9</constant>
        <linear>0.01</linear>
        <quadratic>0.001</quadratic>
      </attenuation>
      <direction>-0.5 0.1 -0.9</direction>
    </light>

    <model name="ground_plane">
      <static>true</static>
      <link name="link">
        <collision name="collision">
          <geometry>
            <box>
              <size>20 20 0.1</size>
            </box>
          </geometry>
        </collision>
        <visual name="visual">
          <geometry>
            <box>
              <size>20 20 0.1</size>
            </box>
          </geometry>
        </visual>
      </link>
    </model>

    <model name="cameras_alone">
      <pose>2.5 0 1.5 0 0.0 3.14</pose>
      <link name="link">
        <pose>0.05 0.05 0.05 0 0 0</pose>
        <sensor name="depth_camera1" type="depth_camera">
          <update_rate>10</update_rate>
          <topic>depth_camera</topic>
          <camera>
            <horizontal_fov>1.05</horizontal_fov>
            <image>
              <width>320</width>
              <height>240</height>
              <format>R_FLOAT32</format>
            </image>
            <clip>
              <near>0.1</near>
              <far>10.0</far>
            </clip>
          </camera>
          </sensor>
      </link>
      <static>true</static>
    </model>

  </world>
</sdf>
  1. ign gazebo -v4 gui_leak.sdf -r -s
  2. ign gui -v4 -s ImageDisplay
  3. Watch memory consumption of GUI in e.g. htop

Analysis

This happens in GUI 4.4.0, but not in 4.3.0. My guess is that #212 is the problematic commit, but I haven't found a way to use git bisect with Ignition (it jumps between gui 3 and gui 4 versions, which is not really easy to test).

This happens for depth and thermal cameras. It does not happen for RGB cameras.

@peci1 peci1 added the bug Something isn't working label Dec 23, 2021
@iche033
Copy link
Contributor

iche033 commented Dec 23, 2021

could be fixed by #287

@peci1
Copy link
Contributor Author

peci1 commented Dec 26, 2021

Probably not everything is fixed by #287 - 4.6.0 is also affected...

@peci1
Copy link
Contributor Author

peci1 commented Dec 26, 2021

6.2.0 is not affected

@peci1
Copy link
Contributor Author

peci1 commented Dec 26, 2021

5.3.0 is also good, so this is only a problem in Dome.

@iche033
Copy link
Contributor

iche033 commented Jan 4, 2022

hmm I see that ign-gui 4.6.0 was released with the mem leak fix. If it's still leaking, the bug could be somewhere else. https://github.com/ignitionrobotics/ign-gui/pull/340/files#diff-c275fec9c453c2a42515bc5ab47e30fa4130ff1426aef2f6e000a9a34e122cb8R19

@peci1
Copy link
Contributor Author

peci1 commented Jan 4, 2022

As said earlier, ImageDisplay started leaking with non-RGB images in 4.4.0 (the big refactoring to start using convertToRgb()).

@darksylinc any idea why is it still leaking even using 4.6.0 with #212 merged?

@darksylinc
Copy link
Contributor

darksylinc commented Jan 5, 2022

  1. Are we sure 4.6.0 is using that fix?
  2. I remember ign/src/ign-common/graphics/src/Image.cc having a ton of leaks which I fixed in ign-common#240 (it was also causing slight mem. corruption) when manipulating non-RGB images. It might be related to that? I don't know if those classes are being used by ign-gui (or maybe the leak is in the server? I also don't know if the server is using that class)

@darksylinc
Copy link
Contributor

Yep, ign-gui is definitely using common::Image and look at that! It's in ImageDisplay!

That's most likely the cause.

@peci1
Copy link
Contributor Author

peci1 commented Jan 5, 2022

#212 is definitely merged in gui 4.6.0. However, the leak-fixing ign-common PR you mention was against ign-common4, while Dome still uses ign-common3. I don't see anything in the changelog that would suggest the PR was backported. Good news is ign-common3 is still alive and not EOLed, so if the bug is really there, fixing it could even fix Dome =) I'll give it a try to backport the fix.

@peci1
Copy link
Contributor Author

peci1 commented Jan 5, 2022

@darksylinc Thanks a lot for your help. I backported your ign-common fix ( gazebosim/gz-common#287 ) and now memory consumption is stable even in Dome!

@chapulina
Copy link
Contributor

Just catching up with this, can this issue be closed?

@peci1
Copy link
Contributor Author

peci1 commented Jan 25, 2022

Yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants