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

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

Merged
merged 8 commits into from
Oct 3, 2022

Conversation

iuhilnehc-ynos
Copy link

@iuhilnehc-ynos iuhilnehc-ynos commented Sep 24, 2022

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 Build Status

…ugin

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@gbiggs
Copy link
Contributor

gbiggs commented Sep 25, 2022

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

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos
Copy link
Author

@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>
Copy link

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

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 Show resolved Hide resolved
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos
Copy link
Author

TRY CI BUT WITHOUT MACOS:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Chen Lihui added 2 commits September 27, 2022 11:27
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Chen Lihui added 2 commits September 27, 2022 14:54
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos
Copy link
Author

iuhilnehc-ynos commented Sep 27, 2022

  • Windows Build Status (unrelated to current PR)

@gbiggs gbiggs merged commit c427a50 into ros:rolling Oct 3, 2022
@gbiggs
Copy link
Contributor

gbiggs commented Oct 3, 2022

Thanks for the contribution!

@tylerjw
Copy link

tylerjw commented Oct 21, 2022

@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
Copy link
Author

iuhilnehc-ynos commented Oct 24, 2022

@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.

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.

None yet

4 participants