Skip to content

fix(logger,config_watcher): align loader-lock leak paths#76

Merged
tkhquang merged 2 commits intomainfrom
fix/noexcept-leak-alignment
Apr 25, 2026
Merged

fix(logger,config_watcher): align loader-lock leak paths#76
tkhquang merged 2 commits intomainfrom
fix/noexcept-leak-alignment

Conversation

@tkhquang
Copy link
Copy Markdown
Owner

@tkhquang tkhquang commented Apr 25, 2026

Summary

Closes #75.

Aligns Logger::shutdown_internal and ~ConfigWatcher loader-lock leak paths with the new (std::nothrow) discipline introduced for HookManager in #71. Both previously used static std::vector + emplace_back, which can throw bad_alloc and turn a noexcept destructor into std::terminate under OOM.

Changes

  • Logger::shutdown_internal: per-call new (std::nothrow) std::shared_ptr<AsyncLogger>(...) heap cell, plus the previously missing pin_current_module() call.
  • ~ConfigWatcher: per-call new (std::nothrow) std::unique_ptr<Impl>(...) heap cell, with m_impl.release() fallback on OOM that leaks the raw pointer without invoking ~Impl.
  • Static asserts on is_nothrow_move_constructible_v for the leak cell types in production sources and matching test files.
  • AGENTS.md thread-safety table updated for Logger and ConfigWatcher rows.
  • CMakeLists.txt version bump 3.2.0 -> 3.2.1.

Test plan

  • mingw-debug: 1028/1028 tests pass.
  • msvc-debug: 231/231 filtered tests pass (Logger + ConfigWatcher + HookManager suites).
  • Existing ConfigWatcherLoaderLockTest.MultipleLoaderLockTeardownsAreSafe exercises the new heap cell path.

Summary by CodeRabbit

  • Release

    • Version 3.2.1 released with enhanced memory management and improved reliability during lifecycle transitions.
  • Documentation

    • Updated thread safety model documentation to reflect refined shutdown and teardown behavior across core components.

…ager nothrow pattern

Replace static-vector emplace_back leaks with per-call new (std::nothrow)
heap cells in Logger::shutdown_internal and ~ConfigWatcher so the
noexcept destructor contract is honestly upheld under OOM rather than
turning a vector growth bad_alloc into std::terminate. Add the
previously missing pin_current_module() call to Logger's leak branch.
ConfigWatcher falls back to m_impl.release() if the heap cell allocation
fails, leaking the raw Impl pointer without invoking ~Impl.

Refs #75
@tkhquang tkhquang self-assigned this Apr 25, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df13c8e5-d51c-4aa3-bde0-6ee7f11301cd

📥 Commits

Reviewing files that changed from the base of the PR and between 6945ea1 and 83b2399.

📒 Files selected for processing (7)
  • AGENTS.md
  • CMakeLists.txt
  • src/config_watcher.cpp
  • src/logger.cpp
  • tests/test_config_watcher.cpp
  • tests/test_logger.cpp
  • tests/test_version.cpp

📝 Walkthrough

Walkthrough

A hardening pass aligns Logger::shutdown_internal and ConfigWatcher::~ConfigWatcher destructors to use per-call nothrow heap allocation instead of potentially-throwing vector operations, ensuring they honor their noexcept contract under OOM conditions. Version is bumped to 3.2.1 with corresponding test updates. Documentation is revised to reflect the new leak-cell allocation strategy.

Changes

Cohort / File(s) Summary
Version Bump
CMakeLists.txt, tests/test_version.cpp
Project version incremented to 3.2.1; test expectations updated to verify patch version 1 and string "3.2.1".
Logger Leak-Path Hardening
src/logger.cpp, tests/test_logger.cpp
Replaced static vector emplace_back with per-call new (std::nothrow) heap cell; added static_assert enforcing nothrow move-constructibility of std::shared_ptr<AsyncLogger>; updated includes (<vector> removed, <new> and <type_traits> added).
ConfigWatcher Leak-Path Hardening
src/config_watcher.cpp, tests/test_config_watcher.cpp
Switched from static vector leak to per-call nothrow heap-allocated "cell"; added fallback to release() on OOM; introduced static_assert for nothrow move-constructibility of std::unique_ptr<Impl>; updated test comments and added compile-time assertions.
Documentation Update
AGENTS.md
Refined "Thread safety model" table to document the new per-call heap-cell leak strategy for Logger::shutdown_internal and ConfigWatcher, replacing static vector references with nothrow allocation details and OOM fallback behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #74: Introduced the loader-lock leak-on-purpose implementation for ConfigWatcher and Logger that is being refactored into a nothrow-allocation pattern in this PR.
  • PR #71: Established the HookManager nothrow heap-allocation + release() fallback pattern that is now replicated across Logger and ConfigWatcher for consistency.
  • PR #69: Addressed logger shutdown and loader-lock leak handling, providing context for the lifecycle constraints being hardened here.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: aligning loader-lock leak paths in Logger and ConfigWatcher components to use nothrow allocation patterns.
Linked Issues check ✅ Passed The PR fully addresses issue #75 requirements: Logger and ConfigWatcher leak paths now use nothrow new() instead of emplace_back, with release() fallback for OOM, matching the HookManager pattern and honoring noexcept contracts.
Out of Scope Changes check ✅ Passed All changes are directly scoped to requirements from issue #75: refactoring leak paths, adding static_asserts for nothrow move-constructibility, updating documentation, and bumping version. No unrelated modifications detected.

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


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.

CMakeLists.txt project version bumped to 3.2.1; bring the version
macro asserts in lockstep so the suite stays green.
@tkhquang
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tkhquang tkhquang merged commit 9d023e6 into main Apr 25, 2026
2 checks passed
@tkhquang tkhquang deleted the fix/noexcept-leak-alignment branch April 25, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hardening: align Logger and ConfigWatcher leak-on-purpose paths to nothrow pattern

1 participant