Skip to content

Fix memory leak in ACE::HTTPS::Session_T#2508

Merged
jwillemsen merged 1 commit intoDOCGroup:masterfrom
bear101:fix-https-session-memleak
Mar 1, 2026
Merged

Fix memory leak in ACE::HTTPS::Session_T#2508
jwillemsen merged 1 commit intoDOCGroup:masterfrom
bear101:fix-https-session-memleak

Conversation

@bear101
Copy link
Contributor

@bear101 bear101 commented Feb 28, 2026

Seems close_streams() and close_connection() were incorrectly put in constructor instead of destructor.

Summary by CodeRabbit

  • Refactor
    • Improved resource management in HTTPS session handling for better cleanup timing during object lifecycle.

Seems close_streams() and close_connection() were incorrectly put in
constructor instead of destructor.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffb39cd and 462ea84.

📒 Files selected for processing (1)
  • ACE/protocols/ace/INet/HTTPS_Session.cpp

Walkthrough

Resource cleanup logic in an HTTPS session class was relocated from the constructor to the destructor. Previously, close_streams() and close_connection() were called during object construction; these calls now execute during object destruction instead.

Changes

Cohort / File(s) Summary
Constructor/Destructor Refactoring
ACE/protocols/ace/INet/HTTPS_Session.cpp
Moved resource cleanup calls (close_streams() and close_connection()) from constructor to destructor, deferring cleanup to object destruction rather than construction.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A cleanup tale, so clean and neat,
From birth to death, the circle's complete,
Resources rest 'til the end of the road,
The destructor now bears the load!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Fix memory leak in ACE::HTTPS::Session_T' directly addresses the main objective: moving resource cleanup from constructor to destructor to fix a memory leak in the HTTPS session class.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

@jwillemsen jwillemsen merged commit 71061fa into DOCGroup:master Mar 1, 2026
45 of 48 checks passed
@jwillemsen
Copy link
Member

Thanks!

@bear101 bear101 deleted the fix-https-session-memleak branch March 3, 2026 20:24
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.

2 participants