Skip to content

Fix memory leak in overlapping relocation test functions (#7184)#7187

Merged
hkaiser merged 3 commits intoTheHPXProject:masterfrom
GitMasterJatin:fix/relocation-test-memory-leak
Apr 23, 2026
Merged

Fix memory leak in overlapping relocation test functions (#7184)#7187
hkaiser merged 3 commits intoTheHPXProject:masterfrom
GitMasterJatin:fix/relocation-test-memory-leak

Conversation

@GitMasterJatin
Copy link
Copy Markdown
Contributor

Summary

Fixes #7184

The setup<T>() helper in relocation test files allocates two buffers (mem1, mem2) and returns them as std::pair<T*, T*>. Non-overlapping tests correctly free both, but overlapping tests only use one buffer for in-place relocation — the second buffer was captured as a discarded binding (___) and never freed, leaking N * sizeof(T) bytes per overlapping test block.

Changes

Introduced a setup_single<T>() helper that allocates only a single buffer for overlapping tests, then updated all overlapping test blocks to use it. This eliminates:

  • The unnecessary second allocation
  • The silent memory leak
  • The misleading discarded binding pattern

Affected files (6 files, 20 leak sites fixed):

File Leak sites
uninitialized_relocate.cpp 4 (test_overlapping x 3, test_right_overlapping x 1)
uninitialized_relocaten.cpp 4 (test_overlapping x 3, test_right_overlapping x 1)
uninitialized_relocate_backward.cpp 3 (test_overlapping x 3)
uninitialized_relocate_sender.cpp 3 (test_overlapping x 3)
uninitialized_relocaten_sender.cpp 3 (test_overlapping x 3)
uninitialized_relocate_backward_sender.cpp 3 (test_overlapping x 3)

Testing

Built and ran all 3 available relocation test targets locally (macOS, Apple Clang, Debug build):

$ ./bin/uninitialized_relocate_test --hpx:threads=2          # EXIT_CODE=0
$ ./bin/uninitialized_relocaten_test --hpx:threads=2         # EXIT_CODE=0
$ ./bin/uninitialized_relocate_backward_test --hpx:threads=2 # EXIT_CODE=0

All tests pass. The sender test targets were not available in this build configuration but the changes are structurally identical.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 14, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@StellarBot
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a memory leak in overlapping relocation test blocks by eliminating an unnecessary second allocation from the shared test setup helper.

Changes:

  • Added a setup_single<T>() helper that allocates/initializes only one buffer for overlapping relocation tests.
  • Updated overlapping and right-overlapping test blocks to use setup_single<T>() instead of setup<T>() (which allocates two buffers).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
libs/core/algorithms/tests/unit/algorithms/uninitialized_relocate.cpp Adds setup_single and updates overlapping/right-overlapping tests to use it, removing the leaked second buffer.
libs/core/algorithms/tests/unit/algorithms/uninitialized_relocaten.cpp Adds setup_single and updates overlapping/right-overlapping tests to use it, removing the leaked second buffer.
libs/core/algorithms/tests/unit/algorithms/uninitialized_relocate_backward.cpp Adds setup_single and updates overlapping tests to use it, removing the leaked second buffer.
libs/core/algorithms/tests/unit/algorithms/uninitialized_relocate_sender.cpp Adds setup_single and updates overlapping sender tests to use it, removing the leaked second buffer.
libs/core/algorithms/tests/unit/algorithms/uninitialized_relocaten_sender.cpp Adds setup_single and updates overlapping sender tests to use it, removing the leaked second buffer.
libs/core/algorithms/tests/unit/algorithms/uninitialized_relocate_backward_sender.cpp Adds setup_single and updates overlapping sender tests to use it, removing the leaked second buffer.

Comment thread libs/core/algorithms/tests/unit/algorithms/uninitialized_relocate.cpp Outdated
@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 14, 2026

@GitMasterJatin please pay attention to the CIs, e.g., the clang-format issues reported.

…ct#7184)

The setup<T>() helper allocates two buffers (mem1, mem2), but overlapping
test blocks only use one buffer for in-place relocation. The second buffer
was captured as a discarded binding (___) and never freed, leaking
N * sizeof(T) bytes per overlapping test block.

This commit introduces a setup_single<T>() helper that allocates only one
buffer, and updates all overlapping test blocks across 6 files to use it:

- uninitialized_relocate.cpp (4 sites)
- uninitialized_relocaten.cpp (4 sites)
- uninitialized_relocate_backward.cpp (3 sites)
- uninitialized_relocate_sender.cpp (3 sites)
- uninitialized_relocaten_sender.cpp (3 sites)
- uninitialized_relocate_backward_sender.cpp (3 sites)

Closes TheHPXProject#7184
@GitMasterJatin GitMasterJatin force-pushed the fix/relocation-test-memory-leak branch from 45a2f56 to dd6b670 Compare April 22, 2026 11:14
@GitMasterJatin
Copy link
Copy Markdown
Contributor Author

@GitMasterJatin please pay attention to the CIs, e.g., the clang-format issues reported.

Done Sir moved all shared test infrastructure (counted_struct, type aliases, clear(), setup(), and setup_single()) into a new header uninitialized_relocate_test_utils.hpp that is included by all 6 test files. Also fixed the clang-format violations. Force-pushed in dd6b670

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 22, 2026

@GitMasterJatin unfortunately, inspect is still not happy:

/libs/core/algorithms/tests/unit/algorithms/uninitialized_relocate_test_utils.hpp:
    Non-ASCII: (line 55)

The inspect tool flagged 4 Unicode EN DASH (U+2013) characters in the
section-divider comments of uninitialized_relocate_test_utils.hpp.
Replace all occurrences with plain ASCII hyphens to satisfy the
non-ASCII character check.
Copilot AI review requested due to automatic review settings April 23, 2026 13:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

setup() calls std::fill but <algorithm> was not directly included.
The .cpp files no longer include <random>, so std::fill can no longer
rely on transitive includes to pull in <algorithm>.
Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@hkaiser hkaiser added this to the 2.0.0 milestone Apr 23, 2026
@hkaiser hkaiser merged commit a74ea4d into TheHPXProject:master Apr 23, 2026
67 checks passed
@GitMasterJatin
Copy link
Copy Markdown
Contributor Author

LGTM, thanks!

Thank you Sir

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak in overlapping relocation test functions

4 participants