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

orkaudio: Fix leak in ConfigManager #112

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kingster
Copy link
Member

@kingster kingster commented Jun 29, 2022

Leak Info

==16187== 54,336 (224 direct, 54,112 indirect) bytes in 1 blocks are definitely lost in loss record 2,012 of 2,023
==16187==    at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16187==    by 0x4FDF968: xercesc_3_1::MemoryManagerImpl::allocate(unsigned long) (in /usr/lib/x86_64-linux-gnu/libxerces-c-3.1.so)
==16187==    by 0x4F64BE7: xercesc_3_1::XMemory::operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/libxerces-c-3.1.so)
==16187==    by 0x57E3AD9: ConfigManager::Initialize() (ConfigManager.cpp:113)
==16187==    by 0x40F062: MainThread() (OrkAudio.cpp:227)
==16187==    by 0x40C87F: main (OrkAudio.cpp:440)

Attempted Fix

diff --git a/orkbasecxx/ConfigManager.cpp b/orkbasecxx/ConfigManager.cpp
index f0653766..aa16f482 100644
--- a/orkbasecxx/ConfigManager.cpp
+++ b/orkbasecxx/ConfigManager.cpp

@@ -150,11 +150,13 @@ void ConfigManager::Initialize()
 				LOG4CXX_ERROR(LOG.configLog, CStdString("Could not parse config file:") + CONFIG_FILE_NAME);
 				failed = true;
 			}
+			doc->release();
 		}
 		else
 		{
 			LOG4CXX_WARN(LOG.configLog, CStdString("Could not find config file:") + CONFIG_FILE_NAME);
 		}
+		delete m_parser;
 	}
 	catch (const CStdString& e)
 	{

Throws this exception when run without debug mode

Stack trace (most recent call last):
#1    Object "", at 0x9a80ff, in
#0    Object "/usr/lib/x86_64-linux-gnu/libxerces-c-3.1.so", at 0x7f85f4550230, in xercesc_3_1::AbstractDOMParser::cleanUp()
Segmentation fault (Invalid permissions for mapped object [0x7f85f2b52cc8])
Segmentation fault (core dumped)

@kingster
Copy link
Member Author

kingster commented Jun 29, 2022

Fixed by removing

doc->release(); 

Seems like this is not required, delete takes care of that release itself.

Before

==137388== LEAK SUMMARY:
==137388==    definitely lost: 224 bytes in 1 blocks
==137388==    indirectly lost: 51,988 bytes in 248 blocks
==137388==      possibly lost: 395,866 bytes in 2,834 blocks
==137388==    still reachable: 203,256 bytes in 1,013 blocks
==137388==         suppressed: 0 bytes in 0 blocks

After

==136663== LEAK SUMMARY:
==136663==    definitely lost: 0 bytes in 0 blocks
==136663==    indirectly lost: 0 bytes in 0 blocks
==136663==      possibly lost: 278,018 bytes in 2,826 blocks
==136663==    still reachable: 203,294 bytes in 1,013 blocks
==136663==         suppressed: 0 bytes in 0 blocks

possibly lost are mostly other xercesc internal references.

@kingster kingster marked this pull request as ready for review June 29, 2022 20:19
@kingster
Copy link
Member Author

Crashes on orkaudio debug on ubuntu16.04

2022-06-30 02:01:18,439  INFO root:109 - Loaded plugin: /usr/lib/libvoip.so
Stack trace (most recent call last):
#3    Object "", at 0x6e68, in
#2    Object "", at 0x9, in
#1    Object "/lib/x86_64-linux-gnu/ld-2.23.so", at 0x7ffeef1e61df, in
#0    Source "/oreka-src/orkbasecxx/serializers/DomSerializer.cpp", line 185, in FindElementByName [0x7f0bc06d99af]
Segmentation fault (Invalid permissions for mapped object [0x9dce30])
Segmentation fault (core dumped)

@kingster kingster marked this pull request as draft June 29, 2022 20:39
kingster added 2 commits June 30, 2022 02:25
…to fix-leak-config-manager

* 'fix-leak-config-manager' of github.com:voiceip/oreka:
  orkaudio: Fix leak in Opus Header Write (#110)
@kingster
Copy link
Member Author

Not exactly sure, but this looked like a case of shadow variable. Changing it to class reference, both resolves the leak & seg fault.

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.

1 participant