fix(graphic): resolve memory leaks in TestGPUBuffer by Miou-zora · Pull Request #571 · EngineSquared/EngineSquared

Miou-zora · 2026-04-07T08:55:07Z

Pull Request

Description

Fixed memory leaks in test TestGPUBuffer

Related Issues (Put "None" if there are no related issues)

close #563

Type of Change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)

Changes Made

List the main changes in this PR:

Added a dtor to test class

Testing

Describe the tests you ran to verify your changes. Please delete options that are not relevant.

Unit tests pass (xmake test)
Manual testing performed (describe what you tested): run xmake check_leaks -v RenderPassTest

Test Environment

OS: macOS
Compiler: Clang

Screenshots/Videos (Put "None" if there are no related issues)

None

Documentation

Please delete options that are not relevant.

No documentation changes are required

Checklist (Don't delete any options)

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published

Breaking Changes (Put "None" if there are no related issues)

None

Additional Notes (Put "None" if there are no related issues)

None

Summary by CodeRabbit

Bug Fixes
- Improved resource cleanup during texture readback so encoder and readback buffers are released promptly, reducing leak risk and stabilizing memory usage.
Tests
- Strengthened GPU buffer lifecycle in tests: buffers are non-copyable and are reliably destroyed (via destructor), improving test stability and preventing dangling resources.
Chores
- Minor build script whitespace cleanup.

coderabbitai · 2026-04-07T08:55:22Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 361bb4c1-56c7-4eaa-86fa-49876b94ec24

📥 Commits

Reviewing files that changed from the base of the PR and between c0bb6e0 and e89d5d4.

📒 Files selected for processing (1)

tools/xmake/build_documentation.lua

💤 Files with no reviewable changes (1)

tools/xmake/build_documentation.lua

📝 Walkthrough

Walkthrough

Texture readback now releases the command encoder and the readback buffer explicitly; test utilities make TestGPUBuffer non-copyable, add a parameterless Destroy() and a virtual destructor to guarantee buffer release.

Changes

Cohort / File(s)	Summary
Texture Readback Cleanup `src/plugin/graphic/src/resource/Texture.hpp`	`RetrieveImage` now calls `encoder.release()` immediately after `encoder.finish()` and calls `readbackBuffer.release()` before returning, changing resource lifetime handling during readback.
Test GPU Buffer Resource Management `src/plugin/graphic/tests/RenderPassTest.cpp`	`TestGPUBuffer` is made non-copyable (deleted copy ctor/assign); added `void Destroy()`; `Destroy(Engine::Core&) override` delegates to `Destroy()`; added virtual destructor that calls `Destroy()` to ensure buffer release.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

test

Poem

🐇 I nibble bytes and tidy threads,

I free the buffers where leakbeds spread.
Encoder released, readback set free,
Tests breathe easy, memory's key.
A tiny hop — all clean to be.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix(graphic): resolve memory leaks in TestGPUBuffer' directly and specifically describes the main change, aligning with the primary objective of fixing memory leaks in the test class.
Linked Issues check	✅ Passed	The PR addresses the core objective of issue `#563` by adding a destructor to TestGPUBuffer to prevent memory leaks, and also manages encoder and buffer lifetimes in Texture.hpp. Both changes align with eliminating memory leaks so RenderPassTest reports cleanly under memory checkers.
Out of Scope Changes check	✅ Passed	All changes are in scope: TestGPUBuffer refactoring addresses the memory leak issue, Texture.hpp changes improve resource management for the same test, and the trailing blank line removal in build_documentation.lua is an incidental cleanup.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 563-fix-fix-memory-leaks-in-renderpasstest

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

src/plugin/graphic/tests/RenderPassTest.cpp (1)

117-117: Use override on the destructor instead of virtual.

Line 117 should be ~TestGPUBuffer() override for clearer intent and to satisfy the static-analysis warning.

Suggested tweak

-    virtual ~TestGPUBuffer() { Destroy(); }
+    ~TestGPUBuffer() override { Destroy(); }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugin/graphic/tests/RenderPassTest.cpp` at line 117, Change the
destructor of TestGPUBuffer to use override instead of virtual: locate the
destructor declaration "~TestGPUBuffer()" (currently "virtual ~TestGPUBuffer() {
Destroy(); }") and replace the "virtual" specifier with the "override" specifier
so it reads "~TestGPUBuffer() override { Destroy(); }" to express overriding
intent and satisfy static analysis.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/plugin/graphic/tests/RenderPassTest.cpp`:
- Line 117: Change the destructor of TestGPUBuffer to use override instead of
virtual: locate the destructor declaration "~TestGPUBuffer()" (currently
"virtual ~TestGPUBuffer() { Destroy(); }") and replace the "virtual" specifier
with the "override" specifier so it reads "~TestGPUBuffer() override {
Destroy(); }" to express overriding intent and satisfy static analysis.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 773354aa-a67a-4754-8d10-df0aa7be7f1c

📥 Commits

Reviewing files that changed from the base of the PR and between 8ffd287 and 8ffc5e9.

📒 Files selected for processing (2)

src/plugin/graphic/src/resource/Texture.hpp
src/plugin/graphic/tests/RenderPassTest.cpp

…uild

sonarqubecloud · 2026-04-07T14:40:50Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

fix(graphic): resolve memory leaks in Texture and TestGPUBuffer

8ffc5e9

Miou-zora linked an issue Apr 7, 2026 that may be closed by this pull request

[FIX] Fix memory leaks in RenderPassTest #563

Closed

Miou-zora self-assigned this Apr 7, 2026

coderabbitai Bot reviewed Apr 7, 2026

View reviewed changes

Miou-zora added 5 commits April 7, 2026 10:59

chore(graphic): ensure proper destruction of TestGPUBuffer

50bbc25

fix(graphic): update destructor in TestGPUBuffer for proper override

c2b834d

Merge branch 'main' into 563-fix-fix-memory-leaks-in-renderpasstest

c0bb6e0

chore(build): remove optional doxygen dependency from documentation b…

8d25b0f

…uild

build(doxygen): put the task file to original

e89d5d4

Miou-zora merged commit ae7a9cc into main Apr 7, 2026
29 checks passed

Miou-zora deleted the 563-fix-fix-memory-leaks-in-renderpasstest branch April 7, 2026 20:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(graphic): resolve memory leaks in TestGPUBuffer#571

fix(graphic): resolve memory leaks in TestGPUBuffer#571
Miou-zora merged 6 commits into
mainfrom
563-fix-fix-memory-leaks-in-renderpasstest

Miou-zora commented Apr 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 7, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

sonarqubecloud Bot commented Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Miou-zora commented Apr 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Description

Related Issues (Put "None" if there are no related issues)

Type of Change

Changes Made

Testing

Test Environment

Screenshots/Videos (Put "None" if there are no related issues)

Documentation

Checklist (Don't delete any options)

Breaking Changes (Put "None" if there are no related issues)

Additional Notes (Put "None" if there are no related issues)

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 7, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Miou-zora commented Apr 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 7, 2026 •

edited

Loading