Ensure cancellation::none implementation does not allocate#56
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes the cancellation class to prevent memory allocations in the cancellation::none() singleton. The none() singleton uses a no_destroy wrapper whose destructor never runs, which would cause any member allocations to leak in debug builds on some platforms. The solution wraps m_listeners in std::optional to defer allocation until a listener is actually added, which never occurs for the none() singleton.
Changes:
- Wrapped
m_listenersmember instd::optionalto enable lazy initialization - Updated all accesses to
m_listenersto use optional's->operator after checking or emplacing - Made
throw_if_cancellation_requested()const for improved const-correctness
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
56fa721 to
2db4218
Compare
2db4218 to
40f08b4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This is a fix on top of #26.
This pull request introduces a new standalone test executable for Windows platforms to detect memory leaks related to the
cancellation::none()implementation. The test leverages_CrtDumpMemoryLeaksto ensure that the no-destroy pattern used in cancellation logic does not result in leaks, especially when heap allocations are involved. The test is only run in debug builds, and is integrated into the build and test system for Windows.New memory leak detection test for cancellation logic:
cancellation_leak_testthat verifiescancellation::none()does not cause memory leaks by invoking_CrtDumpMemoryLeaksduring test execution (Source/Windows.Test/Threading/CancellationMemoryLeakTest.cpp).CMakeLists.txtto build and run the executable as a CTest, conditional on Windows and test configuration.