make the meta-object alive in the lifecycle of the registered plugin by iuhilnehc-ynos · Pull Request #201 · ros/class_loader

iuhilnehc-ynos · 2022-09-24T17:27:13Z

This PR is to address the memory leak about the meta-object is not destroyed during a process exit if using ClassLoader.

memory leak log

chenlh test $ valgrind --leak-check=full ./class_loader_unique_ptr_test --gtest_filter="*.basicLoad"
==63164== Memcheck, a memory error detector
==63164== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==63164== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==63164== Command: ./class_loader_unique_ptr_test --gtest_filter=*.basicLoad
==63164== 
Note: Google Test filter = *.basicLoad
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ClassLoaderUniquePtrTest
[ RUN      ] ClassLoaderUniquePtrTest.basicLoad
Meow
*******************************************************************************
*****                 class_loader impl DEBUG INFORMATION                 *****
*******************************************************************************
OPEN LIBRARIES IN MEMORY:
--------------------------------------------------------------------------------
Open library 0 = libclass_loader_TestPlugins1.so (handle = 0x4ede240)
METAOBJECTS (i.e. FACTORIES) IN MEMORY:
--------------------------------------------------------------------------------
Metaobject 0 (ptr = 0x4edf050):
 TypeId = N12class_loader4impl10MetaObjectI3Cat4BaseEE
 Associated Library = libclass_loader_TestPlugins1.so
 Associated Loader 0 = 0x1ffeff8000
--------------------------------------------------------------------------------
Metaobject 1 (ptr = 0x4edf6b0):
 TypeId = N12class_loader4impl10MetaObjectI3Cow4BaseEE
 Associated Library = libclass_loader_TestPlugins1.so
 Associated Loader 0 = 0x1ffeff8000
--------------------------------------------------------------------------------
Metaobject 2 (ptr = 0x4edec70):
 TypeId = N12class_loader4impl10MetaObjectI3Dog4BaseEE
 Associated Library = libclass_loader_TestPlugins1.so
 Associated Loader 0 = 0x1ffeff8000
--------------------------------------------------------------------------------
Metaobject 3 (ptr = 0x4edf380):
 TypeId = N12class_loader4impl10MetaObjectI4Duck4BaseEE
 Associated Library = libclass_loader_TestPlugins1.so
 Associated Loader 0 = 0x1ffeff8000
--------------------------------------------------------------------------------
Metaobject 4 (ptr = 0x4edf9e0):
 TypeId = N12class_loader4impl10MetaObjectI5Sheep4BaseEE
 Associated Library = libclass_loader_TestPlugins1.so
 Associated Loader 0 = 0x1ffeff8000
--------------------------------------------------------------------------------
********************************** END DEBUG **********************************
*******************************************************************************

[       OK ] ClassLoaderUniquePtrTest.basicLoad (161 ms)
[----------] 1 test from ClassLoaderUniquePtrTest (167 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (196 ms total)
[  PASSED  ] 1 test.
==63164== 
==63164== HEAP SUMMARY:
==63164==     in use at exit: 1,040 bytes in 20 blocks
==63164==   total heap usage: 502 allocs, 482 frees, 129,843 bytes allocated
==63164== 
==63164== 208 (16 direct, 192 indirect) bytes in 1 blocks are definitely lost in loss record 16 of 20
==63164==    at 0x483DE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==63164==    by 0x48A81A0: ???
==63164==    by 0x48A7667: ???
==63164==    by 0x48A7E35: ???
==63164==    by 0x48A7E7F: ???
==63164==    by 0x4011B99: call_init.part.0 (dl-init.c:72)
==63164==    by 0x4011CA0: call_init (dl-init.c:30)
==63164==    by 0x4011CA0: _dl_init (dl-init.c:119)
==63164==    by 0x4C8E984: _dl_catch_exception (dl-error-skeleton.c:182)
==63164==    by 0x40160CE: dl_open_worker (dl-open.c:758)
==63164==    by 0x4C8E927: _dl_catch_exception (dl-error-skeleton.c:208)
==63164==    by 0x4015609: _dl_open (dl-open.c:837)
==63164==    by 0x4EB034B: dlopen_doit (dlopen.c:66)
==63164== 
==63164== 208 (16 direct, 192 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 20
==63164==    at 0x483DE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==63164==    by 0x48A8436: ???
==63164==    by 0x48A7815: ???
==63164==    by 0x48A7E41: ???
==63164==    by 0x48A7E7F: ???
==63164==    by 0x4011B99: call_init.part.0 (dl-init.c:72)
==63164==    by 0x4011CA0: call_init (dl-init.c:30)
==63164==    by 0x4011CA0: _dl_init (dl-init.c:119)
==63164==    by 0x4C8E984: _dl_catch_exception (dl-error-skeleton.c:182)
==63164==    by 0x40160CE: dl_open_worker (dl-open.c:758)
==63164==    by 0x4C8E927: _dl_catch_exception (dl-error-skeleton.c:208)
==63164==    by 0x4015609: _dl_open (dl-open.c:837)
==63164==    by 0x4EB034B: dlopen_doit (dlopen.c:66)
==63164== 
==63164== 208 (16 direct, 192 indirect) bytes in 1 blocks are definitely lost in loss record 18 of 20
==63164==    at 0x483DE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==63164==    by 0x48A86CC: ???
==63164==    by 0x48A79C3: ???
==63164==    by 0x48A7E4D: ???
==63164==    by 0x48A7E7F: ???
==63164==    by 0x4011B99: call_init.part.0 (dl-init.c:72)
==63164==    by 0x4011CA0: call_init (dl-init.c:30)
==63164==    by 0x4011CA0: _dl_init (dl-init.c:119)
==63164==    by 0x4C8E984: _dl_catch_exception (dl-error-skeleton.c:182)
==63164==    by 0x40160CE: dl_open_worker (dl-open.c:758)
==63164==    by 0x4C8E927: _dl_catch_exception (dl-error-skeleton.c:208)
==63164==    by 0x4015609: _dl_open (dl-open.c:837)
==63164==    by 0x4EB034B: dlopen_doit (dlopen.c:66)
==63164== 
==63164== 208 (16 direct, 192 indirect) bytes in 1 blocks are definitely lost in loss record 19 of 20
==63164==    at 0x483DE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==63164==    by 0x48A8962: ???
==63164==    by 0x48A7B71: ???
==63164==    by 0x48A7E59: ???
==63164==    by 0x48A7E7F: ???
==63164==    by 0x4011B99: call_init.part.0 (dl-init.c:72)
==63164==    by 0x4011CA0: call_init (dl-init.c:30)
==63164==    by 0x4011CA0: _dl_init (dl-init.c:119)
==63164==    by 0x4C8E984: _dl_catch_exception (dl-error-skeleton.c:182)
==63164==    by 0x40160CE: dl_open_worker (dl-open.c:758)
==63164==    by 0x4C8E927: _dl_catch_exception (dl-error-skeleton.c:208)
==63164==    by 0x4015609: _dl_open (dl-open.c:837)
==63164==    by 0x4EB034B: dlopen_doit (dlopen.c:66)
==63164== 
==63164== 208 (16 direct, 192 indirect) bytes in 1 blocks are definitely lost in loss record 20 of 20
==63164==    at 0x483DE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==63164==    by 0x48A8BF8: ???
==63164==    by 0x48A7D1F: ???
==63164==    by 0x48A7E65: ???
==63164==    by 0x48A7E7F: ???
==63164==    by 0x4011B99: call_init.part.0 (dl-init.c:72)
==63164==    by 0x4011CA0: call_init (dl-init.c:30)
==63164==    by 0x4011CA0: _dl_init (dl-init.c:119)
==63164==    by 0x4C8E984: _dl_catch_exception (dl-error-skeleton.c:182)
==63164==    by 0x40160CE: dl_open_worker (dl-open.c:758)
==63164==    by 0x4C8E927: _dl_catch_exception (dl-error-skeleton.c:208)
==63164==    by 0x4015609: _dl_open (dl-open.c:837)
==63164==    by 0x4EB034B: dlopen_doit (dlopen.c:66)
==63164== 
==63164== LEAK SUMMARY:
==63164==    definitely lost: 80 bytes in 5 blocks
==63164==    indirectly lost: 960 bytes in 15 blocks
==63164==      possibly lost: 0 bytes in 0 blocks
==63164==    still reachable: 0 bytes in 0 blocks
==63164==         suppressed: 0 bytes in 0 blocks
==63164== 
==63164== For lists of detected and suppressed errors, rerun with: -s
==63164== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 0 from 0)

Signed-off-by: Chen Lihui lihui.chen@sony.com

Linux

…ugin Signed-off-by: Chen Lihui <lihui.chen@sony.com>

gbiggs · 2022-09-25T23:06:02Z

Can you please provide more information about the bug this PR will address?

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

iuhilnehc-ynos · 2022-09-26T02:46:19Z

@gbiggs

Thanks for your reply. I have updated the description in #201 (comment). In order to not make the relative issue complicated, I used the existing test case to show the problem.

… utest Signed-off-by: Chen Lihui <lihui.chen@sony.com>

fujitatomoya

Introducing unique_ptr can keep the meta object as long as proxy object is alive and guarantee to delete the object, this makes sense to address the issue.

src/class_loader_core.cpp

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

iuhilnehc-ynos · 2022-09-27T02:22:39Z

TRY CI BUT WITHOUT MACOS:

Linux
Linux-aarch64
Windows

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

iuhilnehc-ynos · 2022-09-27T09:48:21Z

Windows (unrelated to current PR)

gbiggs · 2022-10-03T22:55:54Z

Thanks for the contribution!

tylerjw · 2022-10-21T15:16:44Z

@iuhilnehc-ynos I was hopeful that your much simpler change here would fix the bugs found by asan... but it unfortunately does not: https://github.com/tylerjw/class_loader/actions/runs/3298507574/jobs/5440716254

iuhilnehc-ynos · 2022-10-24T01:34:32Z

@tylerjw

Thanks for your information, but I think this PR is to address the memory leak about the meta-object is not destroyed during a process exit if using ClassLoader mentioned at #201 (comment). To make the sanitizer happy, I have created a new PR below for it.

make the meta-object alive only in the lifecycle of the registered pl…

35a23dd

…ugin Signed-off-by: Chen Lihui <lihui.chen@sony.com>

remove unuseful variable passed in the lambda

de7241a

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

iuhilnehc-ynos marked this pull request as ready for review September 26, 2022 02:46

iuhilnehc-ynos mentioned this pull request Sep 26, 2022

purge meta objects within the graveyard if the library is unassociated #200

Closed

add extra tests to make test cases exists in both unique ptr test and…

2eb0eb7

… utest Signed-off-by: Chen Lihui <lihui.chen@sony.com>

fujitatomoya reviewed Sep 26, 2022

View reviewed changes

src/class_loader_core.cpp Show resolved Hide resolved

use the original mutex rather than import a new mutex

1cc61a5

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

Chen Lihui added 2 commits September 27, 2022 11:27

update document comment

6ef26d5

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

symbol visibility for Windows

57ef608

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

iuhilnehc-ynos force-pushed the topic-meta-object-lifecycle branch from 5cd61f2 to 02e1a8a Compare September 27, 2022 06:52

Chen Lihui added 2 commits September 27, 2022 14:54

add extra document comment

02e1a8a

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

update doc

c44e59e

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

gbiggs approved these changes Oct 3, 2022

View reviewed changes

fujitatomoya approved these changes Oct 3, 2022

View reviewed changes

gbiggs merged commit c427a50 into ros:rolling Oct 3, 2022

gbiggs mentioned this pull request Oct 3, 2022

Fix memory leak: meta objects not destroyed #202

Closed

iuhilnehc-ynos mentioned this pull request Oct 24, 2022

make sanitizer happy #205

Merged

MichaelOrlov mentioned this pull request Mar 19, 2025

Calling rosbag2_cpp::Reader::open from multiple threads leads to crashes ros2/rosbag2#1925

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make the meta-object alive in the lifecycle of the registered plugin#201

make the meta-object alive in the lifecycle of the registered plugin#201
gbiggs merged 8 commits intoros:rollingfrom
iuhilnehc-ynos:topic-meta-object-lifecycle

iuhilnehc-ynos commented Sep 24, 2022 •

edited

Loading

Uh oh!

gbiggs commented Sep 25, 2022 •

edited

Loading

Uh oh!

iuhilnehc-ynos commented Sep 26, 2022

Uh oh!

fujitatomoya left a comment

Uh oh!

Uh oh!

iuhilnehc-ynos commented Sep 27, 2022

Uh oh!

iuhilnehc-ynos commented Sep 27, 2022 •

edited

Loading

Uh oh!

gbiggs commented Oct 3, 2022

Uh oh!

tylerjw commented Oct 21, 2022

Uh oh!

iuhilnehc-ynos commented Oct 24, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

iuhilnehc-ynos commented Sep 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gbiggs commented Sep 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iuhilnehc-ynos commented Sep 26, 2022

Uh oh!

fujitatomoya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iuhilnehc-ynos commented Sep 27, 2022

Uh oh!

iuhilnehc-ynos commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gbiggs commented Oct 3, 2022

Uh oh!

tylerjw commented Oct 21, 2022

Uh oh!

iuhilnehc-ynos commented Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iuhilnehc-ynos commented Sep 24, 2022 •

edited

Loading

gbiggs commented Sep 25, 2022 •

edited

Loading

iuhilnehc-ynos commented Sep 27, 2022 •

edited

Loading

iuhilnehc-ynos commented Oct 24, 2022 •

edited

Loading