[jira] [Comment Edited] (ZOOKEEPER-5103) Address multiple memory leaks identified by static analysis

["Andrew Kyle Purtell (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-5103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18127304#comment-18127304 ] 

Andrew Kyle Purtell edited comment on ZOOKEEPER-5103 at 10/15/26 2:09 AM:
--------------------------------------------------------------------------

Asynchronous client APIs lacked an explicit ownership contract. Calling applications had no clear guidance whether an operational failure required them to release their completion context data or whether the library would dispatch a completion callback reporting the error. Furthermore, internal completion queues, serialized request buffers, and transport state structures were routinely abandoned during network disconnects or handle closure.

The remediation addresses these issues without modifying public API or ABI. When an asynchronous invocation returns a status code other than {{{}ZOK{}}}, the library guarantees that the completion callback will never be registered or invoked, leaving context ownership exclusively with the caller. Request buffer allocation and serialization precede completion registration. This eliminates indeterminate error states such as {{ZMARSHALLINGERROR}} after completion registration. Cyrus SASL callback arrays, context blocks, credential secrets, and copied strings are consolidated into a single contiguous allocation. Applications continue releasing callback structures via standard `free()` as documented. The multithreaded completion thread and IO thread shutdown sequence is improved so that the completion queue is drained completely before thread termination.

The work here also integrates AddressSanitizer and LeakSanitizer configurations into Autotools ({{{}--enable-asan{}}}) and Maven ({{{}-Pc-test-asan{}}}). Validation demonstrates no leaked bytes and no sanitizer findings. The library compiles cleanly under {{-Werror}} across Linux (GCC 11) and macOS (Apple Clang 17) in both Autotools and CMake environments.

Dynamic profiling of {{master}} under AddressSanitizer and LeakSanitizer initially identified 45,578 bytes leaked across 178 allocations alongside a test-side global buffer overflow. Following these fixes, both the single-threaded ({{{}zktest-st{}}}) and multi-threaded ({{{}zktest-mt{}}}) CppUnit test suites pass with 100% success 49/49 and 97/97 tests respectively and no sanitizer findings.


was (Author: apurtell):
Asynchronous client APIs lacked an explicit ownership contract. Calling applications had no clear guidance whether an operational failure required them to release their completion context data or whether the library would dispatch a completion callback reporting the error. Furthermore, internal completion queues, serialized request buffers, and transport state structures were routinely abandoned during network disconnects or handle closure.

The remediation addresses these issues without modifying public API or ABI. When an asynchronous invocation returns a status code other than {{{}ZOK{}}}, the library guarantees that the completion callback will never be registered or invoked, leaving context ownership exclusively with the caller. Request buffer allocation and serialization precede completion registration. This eliminates indeterminate error states such as {{ZMARSHALLINGERROR}} after completion registration and guarantees infallible enqueueing. Cyrus SASL callback arrays, context blocks, credential secrets, and copied strings are consolidated into a single contiguous allocation. Applications continue releasing callback structures via standard `free()` as documented. The multithreaded completion thread and IO thread shutdown sequence is improved so that the completion queue is drained completely before thread termination.

The work here also integrates AddressSanitizer and LeakSanitizer configurations into Autotools ({{{}--enable-asan{}}}) and Maven ({{{}-Pc-test-asan{}}}). Validation demonstrates no leaked bytes and no sanitizer findings. The library compiles cleanly under {{-Werror}} across Linux (GCC 11) and macOS (Apple Clang 17) in both Autotools and CMake environments.

Dynamic profiling of {{master}} under AddressSanitizer and LeakSanitizer initially identified 45,578 bytes leaked across 178 allocations alongside a test-side global buffer overflow. Following these fixes, both the single-threaded ({{{}zktest-st{}}}) and multi-threaded ({{{}zktest-mt{}}}) CppUnit test suites pass with 100% success 49/49 and 97/97 tests respectively and no sanitizer findings.

> Address multiple memory leaks identified by static analysis
> -----------------------------------------------------------
>
>                 Key: ZOOKEEPER-5103
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-5103
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: c client
>            Reporter: Andrew Kyle Purtell
>            Assignee: Andrew Kyle Purtell
>            Priority: Major
>
> The C client leaks memory across multiple error handling paths, request serialization routines, and handle close sequences. A cluster of open JIRA issues (ZOOKEEPER-4975, ZOOKEEPER-4999, ZOOKEEPER-5000, ZOOKEEPER-5004, ZOOKEEPER-5005, ZOOKEEPER-5006, ZOOKEEPER-1985, ZOOKEEPER-2882, and ZOOKEEPER-3868) all have the same essential root cause, which is an ambiguous resource ownership contract (or lack thereof) between the client library and application code during failure modes and connection termination.
> Dynamic analysis of master (commit 48f7cb00e) under AddressSanitizer and LeakSanitizer reports 45,578 bytes leaked across 178 allocations in the CppUnit test harness.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)