memory leaks in src/xml_wrapper.cpp

[yxiang92128]

xml_wrappper_leak_patch.txt valgrind reports the following leaks in src/xml_wrapper.cpp There are three leaks and I’ve provided tentative patches for #1 and #2 as shown in a separate patch file.

1. Leak #1 in xml_text_reader_wrapper::get_value()

==31295== 69,780 bytes in 2,326 blocks are definitely lost in loss record 389 of 394 ==31295== at 0x4C29110: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==31295== by 0x80D1B68: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4) ==31295== by 0x918422B: azure::storage::core::xml::xml_text_reader_wrapper::get_value() (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x91862AC: azure::storage::core::xml::xml_reader::get_current_element_text() (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x91B2C62: azure::storage::protocol::list_blobs_reader::handle_element(std::string const&) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x9186A88: azure::storage::core::xml::xml_reader::parse() (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x932B1E6: azure::storage::cloud_blob_container::list_blobs_segmented_async(std::string const&, bool, azure::storage::blob_listing_details::values, int, azure::storage::continuation_token const&, azure::storage::blob_request_options const&, azure::storage::operation_context) const::{lambda(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context)#1}::operator()(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) const [clone .isra.1026] (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x932C23C: std::_Function_handler<pplx::task<azure::storage::result_segment > (web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context), azure::storage::cloud_blob_container::list_blobs_segmented_async(std::string const&, bool, azure::storage::blob_listing_details::values, int, azure::storage::continuation_token const&, azure::storage::blob_request_options const&, azure::storage::operation_context) const::{lambda(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context)#1}>::_M_invoke(std::_Any_data const&, web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x9339D14: azure::storage::core::storage_command<azure::storage::result_segment >::postprocess_response(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x922F811: azure::storage::core::executor_impl::execute_async(std::shared_ptr, azure::storage::request_options const&, azure::storage::operation_context)::{lambda()#1}::operator()() const::{lambda(pplx::task)#2}::operator()(web::http::http_response) const (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x923FD65: pplx::details::_PPLTaskHandle<unsigned char, pplx::task::_ContinuationTaskHandle<web::http::http_response, void, azure::storage::core::executor_impl::execute_async(std::shared_ptr, azure::storage::request_options const&, azure::storage::operation_context)::{lambda()#1}::operator()() const::{lambda(pplx::task)#2}, std::integral_constant<bool, true>, pplx::details::_TypeSelectorAsyncTask>, pplx::details::_ContinuationTaskHandleBase>::invoke() const (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x51A4F18: pplx::details::_TaskProcHandle::_RunChoreBridge(void*) (pplx.h:113)

2. Leak #2 in xml_text_reader_wrapper::get_local_name() ==31295== 517,950 bytes in 39,694 blocks are definitely lost in loss record 394 of 394 ==31295== at 0x4C29110: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==31295== by 0x80D1B68: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4) ==31295== by 0x918420B: azure::storage::core::xml::xml_text_reader_wrapper::get_local_name() (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x91861FC: azure::storage::core::xml::xml_reader::get_current_element_name() (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x9186AA3: azure::storage::core::xml::xml_reader::parse() (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x932B1E6: azure::storage::cloud_blob_container::list_blobs_segmented_async(std::string const&, bool, azure::storage::blob_listing_details::values, int, azure::storage::continuation_token const&, azure::storage::blob_request_options const&, azure::storage::operation_context) const::{lambda(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context)#1}::operator()(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) const [clone .isra.1026] (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x932C23C: std::_Function_handler<pplx::task<azure::storage::result_segment > (web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context), azure::storage::cloud_blob_container::list_blobs_segmented_async(std::string const&, bool, azure::storage::blob_listing_details::values, int, azure::storage::continuation_token const&, azure::storage::blob_request_options const&, azure::storage::operation_context) const::{lambda(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context)#1}>::_M_invoke(std::_Any_data const&, web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x9339D14: azure::storage::core::storage_command<azure::storage::result_segment >::postprocess_response(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x922F811: azure::storage::core::executor_impl::execute_async(std::shared_ptr, azure::storage::request_options const&, azure::storage::operation_context)::{lambda()#1}::operator()() const::{lambda(pplx::task)#2}::operator()(web::http::http_response) const (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x923FD65: pplx::details::_PPLTaskHandle<unsigned char, pplx::task::_ContinuationTaskHandle<web::http::http_response, void, azure::storage::core::executor_impl::execute_async(std::shared_ptr, azure::storage::request_options const&, azure::storage::operation_context)::{lambda()#1}::operator()() const::{lambda(pplx::task)#2}, std::integral_constant<bool, true>, pplx::details::_TypeSelectorAsyncTask>, pplx::details::_ContinuationTaskHandleBase>::invoke() const (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x51A4F18: pplx::details::_TaskProcHandle::_RunChoreBridge(void) (pplx.h:113) ==31295== by 0x5A8F260: boost::asio::detail::completion_handler<boost::_bi::bind_t<void, void ()(void), boost::_bi::list1<boost::_bi::value<void> > > >::do_complete(boost::asio::detail::task_io_service, boost::asio::detail::task_io_service_operation, boost::system::error_code const&, unsigned long) (in /usr/local/lib/libcpprest.so.2.10)

3. Leak #3 in xml_text_reader_wrapper constructor? (possibly a false positive) ==31295== 220,296 (8,272 direct, 212,024 indirect) bytes in 11 blocks are definitely lost in loss record 392 of 394 ==31295== at 0x4C29110: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==31295== by 0x805C8DE: xmlNewParserCtxt (in /usr/lib64/libxml2.so.2.9.4) ==31295== by 0x806F444: xmlCreatePushParserCtxt (in /usr/lib64/libxml2.so.2.9.4) ==31295== by 0x810EC02: xmlNewTextReader (in /usr/lib64/libxml2.so.2.9.4) ==31295== by 0x81128FA: xmlReaderForMemory (in /usr/lib64/libxml2.so.2.9.4) ==31295== by 0x9184184: azure::storage::core::xml::xml_text_reader_wrapper::xml_text_reader_wrapper(unsigned char const*, unsigned int) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x91875A9: azure::storage::core::xml::xml_reader::initialize(Concurrency::streams::basic_istream) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x932AF52: azure::storage::cloud_blob_container::list_blobs_segmented_async(std::string const&, bool, azure::storage::blob_listing_details::values, int, azure::storage::continuation_token const&, azure::storage::blob_request_options const&, azure::storage::operation_context) const::{lambda(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context)#1}::operator()(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) const [clone .isra.1026] (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x932C23C: std::_Function_handler<pplx::task<azure::storage::result_segment > (web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context), azure::storage::cloud_blob_container::list_blobs_segmented_async(std::string const&, bool, azure::storage::blob_listing_details::values, int, azure::storage::continuation_token const&, azure::storage::blob_request_options const&, azure::storage::operation_context) const::{lambda(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context)#1}>::_M_invoke(std::_Any_data const&, web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x9339D14: azure::storage::core::storage_command<azure::storage::result_segment >::postprocess_response(web::http::http_response const&, azure::storage::request_result const&, azure::storage::core::ostream_descriptor const&, azure::storage::operation_context) (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x922F811: azure::storage::core::executor_impl::execute_async(std::shared_ptr, azure::storage::request_options const&, azure::storage::operation_context)::{lambda()#1}::operator()() const::{lambda(pplx::task)#2}::operator()(web::http::http_response) const (in /usr/local/lib/libazurestorage.so.5.2) ==31295== by 0x923FD65: pplx::details::_PPLTaskHandle<unsigned char, pplx::task::_ContinuationTaskHandle<web::http::http_response, void, azure::storage::core::executor_impl::execute_async(std::shared_ptr, azure::storage::request_options const&, azure::storage::operation_context)::{lambda()#1}::operator()() const::{lambda(pplx::task)#2}, std::integral_constant<bool, true>, pplx::details::_TypeSelectorAsyncTask>, pplx::details::_ContinuationTaskHandleBase>::invoke() const (in /usr/local/lib/libaz

[katmsft]

Thanks for reporting the issue, the issue is caused by following function in xml_wrapper.cpp:

std::string xml_char_to_string(const xmlChar * xml_char)
{
    return std::string(reinterpret_cast<const char*>(xml_char));
}

Basically, the string is copied, which cannot be avoided, during the construction and the source data is not freed. This will be noted to be fixed.

[yxiang92128]

hi,

I see couple of leaks mentioned in the previous report had been fixed in 6.0 and thanks for that. But valgrind had yielded more warnings which might be worth a check. See attached report.

Many thanks,

Yang more_xml_wrapper_mem_leak.docx

[katmsft]

Hi,

The reported warnings by Valgrind are somewhat false alarms that can be ignored. They relate to the internal behavior of Libxml2's optimization of the performance by reusing the existing dictionary/parser/encoding. However, the dictionary/parser/encoding will be preserved until the process is terminated, making Valgrind to report memory leak. There are ways to eliminate the warnings, but none would make it something appropriate for this client library.

Reference:

Function: xmlCleanupParser void xmlCleanupParser (void)

This function name is somewhat misleading. It does not clean up parser state, it cleans up memory allocated by the library itself. It is a cleanup function for the XML library. It tries to reclaim all related global memory allocated for the library processing. It doesn't deallocate any document related memory. One should call xmlCleanupParser() only when the process has finished using the library and all XML/HTML documents built with it. See also xmlInitParser() which has the opposite function of preparing the library for operations. WARNING: if your application is multithreaded or has plugin support calling this may crash the application if another thread or a plugin is still using libxml2. It's sometimes very hard to guess if libxml2 is in use in the application, some libraries or plugins may use it without notice. In case of doubt abstain from calling this function or do it just before calling exit() to avoid leak reports from valgrind !

The cleanup code:

xmlDictCleanup();
xmlCleanupParser();
xmlMemoryDump();
xmlCleanupCharEncodingHandlers();