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

Fix Embag Memory Leaks #32

Merged
merged 7 commits into from
Dec 6, 2021
Merged

Fix Embag Memory Leaks #32

merged 7 commits into from
Dec 6, 2021

Conversation

liambenson
Copy link
Contributor

Fixes: DATA-526

Description of Changes

There was a memory leak resulting from the recent performance changes due to circular shared_ptrs. This fixes the issue by storing weak_ptrs to the vector of RosValues and then having any interfaces that access the RosValues lock the weak_ptr and return a RosValuePointer which stores the locked weak_ptr (which is just a shared_ptr).

Test Plan

Tests + ensured that -fsanitize=address does not show any errors.

@suurjaak
Copy link

+1 to getting the leak fixed. At least on the Python side, the memory leak makes embag unusable for any larger bag.

E.g. just reading through a bag with thousands of messages will eat memory until it's all gone and system freezes:

import time, embag
b = embag.Bag("somebiggerfile.bag")
count = 0
for m in b.read_messages(list(b.topics())):
    count += 1
    if not count % 1000: print(count)
    time.sleep(0.0001)

Same for the results from embag.View, at least when using msg.data().

embag_freeze

@liambenson liambenson merged commit 4396eac into master Dec 6, 2021
@liambenson liambenson deleted the liam/fix_mem_leak branch December 6, 2021 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants