Skip to content

DAOS-19018 kv: fix reference leak in daos_kv2objhandle()#18412

Open
knard38 wants to merge 2 commits into
masterfrom
ckochhof/fix/master/daos-19018/patch-001
Open

DAOS-19018 kv: fix reference leak in daos_kv2objhandle()#18412
knard38 wants to merge 2 commits into
masterfrom
ckochhof/fix/master/daos-19018/patch-001

Conversation

@knard38

@knard38 knard38 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Description

kv_hdl2ptr() increments the dc_kv reference count via daos_hhash_link_lookup(). daos_kv2objhandle() was returning without calling kv_decref(), leaking one reference per call. Add the missing kv_decref() after copying the object handle.

Also add a regression test in the DAOS_KV_API suite that calls daos_kv2objhandle() with both invalid and valid handles, exercises the function repeatedly to surface any leak under ASAN/LeakSanitizer, and verifies the KV handle can be cleanly closed afterwards.

Dependency

This PR depends on PR #18423 (DAOS-19017: fix d_aligned_alloc size constraint violation). Without that fix, ASAN aborts immediately on the misaligned allocation, which prevents it from reaching the leak-detection phase. PR #18423 must be merged first for the ASAN/LeakSanitizer validation below to be reproducible.

Validation

Both runs used daos_test -K with filter kv_obj_handle, built with SANITIZERS=address.

Without the fix — LeakSanitizer detects the dc_kv leak:

==2573095==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 in calloc
    #1 in d_calloc src/gurt/misc.c:126
    #2 in kv_alloc src/client/kv/dc_kv.c:60
    #3 in open_handle_cb src/client/kv/dc_kv.c:130
    ...
    #16 in kv_obj_handle src/tests/suite/daos_kv.c:373

SUMMARY: AddressSanitizer: 17019 byte(s) leaked in 54 allocation(s).

With the fix — no kv_alloc leak, 80 bytes and 1 allocation fewer:

SUMMARY: AddressSanitizer: 16939 byte(s) leaked in 53 allocation(s).

The remaining leaks in both runs (openmpi, isal, hwloc) are pre-existing third-party leaks unrelated to this change.

Steps for the author:

  • Commit message follows the guidelines.
  • Appropriate Features or Test-tag pragmas were used.
  • Appropriate Functional Test Stages were run.
  • At least two positive code reviews including at least one code owner from each category referenced in the PR.
  • Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

  • Gatekeeper requested (daos-gatekeeper added as a reviewer).

@knard38 knard38 self-assigned this Jun 3, 2026
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Ticket title is 'reference leak in daos_kv2objhandle()'
Status is 'In Review'
Labels: 'asan'
https://daosio.atlassian.net/browse/DAOS-19018

@daosbuild3

Copy link
Copy Markdown
Collaborator

@daosbuild3

Copy link
Copy Markdown
Collaborator

@knard38 knard38 force-pushed the ckochhof/fix/master/daos-19018/patch-001 branch from a7a8cc6 to 8cca1f6 Compare June 16, 2026 11:07
@knard38 knard38 changed the base branch from master to ckochhof/fix/master/daos-19017/patch-001 June 16, 2026 11:12
@knard38 knard38 marked this pull request as ready for review June 16, 2026 11:12
@knard38 knard38 requested review from a team as code owners June 16, 2026 11:12
@knard38 knard38 requested review from mchaarawi and wangshilong June 16, 2026 11:13
mchaarawi
mchaarawi previously approved these changes Jun 16, 2026
Base automatically changed from ckochhof/fix/master/daos-19017/patch-001 to master June 16, 2026 14:43
@daltonbohning daltonbohning dismissed mchaarawi’s stale review June 16, 2026 14:43

The base branch was changed.

@daltonbohning daltonbohning requested a review from a team as a code owner June 16, 2026 14:43
@daosbuild3

Copy link
Copy Markdown
Collaborator

knard38 added 2 commits June 17, 2026 06:32
kv_hdl2ptr() increments the dc_kv reference count via
daos_hhash_link_lookup(). daos_kv2objhandle() was returning without
calling kv_decref(), leaking one reference per call. Add the missing
kv_decref() after copying the object handle.

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…e leak

Add kv_obj_handle() to the DAOS_KV_API suite. The test verifies that:
- daos_kv2objhandle() returns DAOS_HDL_INVAL for an invalid handle
- it returns a valid object handle for a valid KV handle
- repeated calls do not accumulate dc_kv references (regression for DAOS-19018)
- the KV handle can be cleanly closed after repeated calls

When built with ASAN/LeakSanitizer, any leaked dc_kv reference will
be reported at process exit.

Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
@knard38 knard38 force-pushed the ckochhof/fix/master/daos-19018/patch-001 branch from 73da934 to 24c8ba7 Compare June 17, 2026 06:32
@knard38

knard38 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Apologies for the force push, but I was not able to properly merge as the initial target branch was landed on master.
The code of the PR was not updated, just cleaning of the git tree of the PR.

@knard38 knard38 requested a review from mchaarawi June 17, 2026 06:33

@wangshilong wangshilong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants