Updated to use RunOnEnd to cleanup

SiddiqSoft · Dec 16, 2024 · 36e52c5 · 36e52c5
1 parent fd5af3a
commit 36e52c5
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 110 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -48,6 +48,9 @@ target_compile_options( ${PROJECT_NAME}
 
 # ____________________________________
 # Dependencies
+cpmaddpackage("gh:siddiqsoft/RunOnEnd#1.3.2")
+target_link_libraries(${PROJECT_NAME} INTERFACE RunOnEnd::RunOnEnd)
+
 
 # ____________________________________
 # Testing

diff --git a/CMakeSettings.json b/CMakeSettings.json
diff --git a/README.md b/README.md
@@ -12,7 +12,6 @@ asynchrony : Add asynchrony to your apps
 - The code here is a set of helpers that utilize the underlying deque, semaphore, mutex features found in std.
 - Be instructive while providing functional code
 - Use only C++20 standard code: jthread, deque, semaphore, barriers and latch
-- No external dependency
 
 # Usage
 
@@ -123,6 +122,7 @@ void main()
 }
 ```
 
+
 ## Implementation note
 In order to use `std::jthread` on Clang 18 and Clang 19, we enable the compiler flag `"CMAKE_CXX_FLAGS": "-fexperimental-library"` in the CMakeLists.txt. This option will show up in your client library under Clang compilers.
 

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -39,11 +39,11 @@ stages:
           poolName: Default
           poolImageName:
           cmake.preset: Windows-Release
-        #Linux:
-        #  osName: Linux
-        #  poolName: Default
-        #  poolImageName:
-        #  cmake.preset: Linux-Release
+        Linux:
+          osName: Linux
+          poolName: Default
+          poolImageName:
+          cmake.preset: Linux-Release
 
     pool:
       name: $(poolName)
@@ -62,13 +62,13 @@ stages:
         versionSpec: '>=6.0.0-0'
         checkLatest: true
 
-    - task: gitversion/setup@3.0.0
+    - task: gitversion/setup@3
       displayName: 'Install GitVersion..'
       inputs:
-        versionSpec: '6.x'
+        versionSpec: '6.0'
         preferLatestVersion: true
 
-    - task: gitversion/execute@3.0.0
+    - task: gitversion/execute@3
       displayName: 'Determine Version..'
       name: version_step
       inputs:
@@ -93,14 +93,14 @@ stages:
       displayName: 'CMake Tests $(cmake.preset)'
       inputs:
         cmakeArgs: '-E chdir $(System.DefaultWorkingDirectory)/build/$(cmake.preset)/tests ctest $(System.DefaultWorkingDirectory)/build/$(cmake.preset)/tests --output-junit $(System.DefaultWorkingDirectory)/build/$(cmake.preset)/tests/results/test_detail.xml --output-on-failure'
-      condition: succeeded()
+      condition: and(succeeded(), startsWith(variables['Agent.OS'], 'Windows'))
       continueOnError: false
 
     # Try for coverage (but only on Linux build machines)
     - task: CMake@1
       displayName: 'CTest Coverage $(cmake.preset) (Linux only)'
       inputs:
-        cmakeArgs: '-E chdir $(System.DefaultWorkingDirectory)/build/$(cmake.preset) ctest --preset $(cmake.preset) --output-junit $(System.DefaultWorkingDirectory)/build/$(cmake.preset)/tests/results/test_detail.xml --output-on-failure  -T Test -T Coverage'
+        cmakeArgs: '-E chdir $(System.DefaultWorkingDirectory)/build/$(cmake.preset) ctest $(System.DefaultWorkingDirectory)/build/$(cmake.preset)/tests --output-junit $(System.DefaultWorkingDirectory)/build/$(cmake.preset)/tests/results/test_detail.xml --output-on-failure  -T Test -T Coverage'
       condition: and(succeeded(), startsWith(variables['Agent.OS'], 'Linux'))
       continueOnError: false
 
@@ -148,11 +148,11 @@ stages:
           poolName: Default
           poolImageName:
           cmake.preset: Windows-Release
-        #Linux:
-        #  osName: Linux
-        #  poolName: Default
-        #  poolImageName:
-        #  cmake.preset: Linux-Release
+        Linux:
+          osName: Linux
+          poolName: Default
+          poolImageName:
+          cmake.preset: Linux-Release
 
     pool:
       name: $(poolName)
@@ -161,13 +161,13 @@ stages:
         - Agent.OS -equals $(osName)
 
     steps:
-    - task: gitversion/setup@3.0.0
+    - task: gitversion/setup@3
       displayName: 'Install GitVersion..'
       inputs:
-        versionSpec: '6.x'
+        versionSpec: '6.0'
         preferLatestVersion: true
 
-    - task: gitversion/execute@3.0.0
+    - task: gitversion/execute@3
       displayName: 'Determine Version..'
       name: version_step
       inputs:
@@ -202,14 +202,14 @@ stages:
       displayName: 'Release to GitHub'
       inputs:
         gitHubConnection: 'github-packages-sqs'
-        repositoryName: 'siddiqsoft/asynchrony'
+        repositoryName: '$(Build.Repository.Name)'
         action: 'create'
         target: '$(Build.SourceVersion)'
         tagSource: 'userSpecifiedTag'
         tag: '$(GitVersion.MajorMinorPatch)'
         title: 'v$(GitVersion.MajorMinorPatch)'
         releaseNotesSource: 'inline'
-        releaseNotesInline: 'Documentation https://github.com/SiddiqSoft/asynchrony'
+        releaseNotesInline: 'Documentation https://github.com/$(Build.Repository.Name)'
         isPreRelease: true
         changeLogCompareToRelease: 'lastFullRelease'
         changeLogType: 'commitBased'

diff --git a/include/siddiqsoft/periodic_worker.hpp b/include/siddiqsoft/periodic_worker.hpp
@@ -45,6 +45,7 @@
 #include <deque>
 #include <semaphore>
 #include <stop_token>
+#include <utility>
 
 
 namespace siddiqsoft
@@ -89,11 +90,11 @@ namespace siddiqsoft
         /// @param interval The interval between each invocation
         periodic_worker(std::function<void()>     c,
                         std::chrono::microseconds interval,
-                        const std::string&        name = {"anonymous-periodic-worker"})
-            : callback(c)
+                        std::string         name = {"anonymous-periodic-worker"})
+            : callback(std::move(c))
             , outstandingCallback(0)
             , invokePeriod(interval)
-            , threadName(name)
+            , threadName(std::move(name))
         {
         }
 

diff --git a/include/siddiqsoft/roundrobin_pool.hpp b/include/siddiqsoft/roundrobin_pool.hpp
@@ -36,6 +36,7 @@
 #ifndef ROUNDROBIN_POOL_HPP
 #define ROUNDROBIN_POOL_HPP
 
+#include <concepts>
 #include "simple_worker.hpp"
 
 
@@ -47,7 +48,7 @@ namespace siddiqsoft
     /// @remarks The number of threads in the pool is determined by the nature of your "work". If you're spending time against db
     /// then you might wish to use more threads as individual queries might take time and hog the thread.
     template <typename T, uint16_t N = 0>
-        requires std::move_constructible<T>
+        requires std::is_move_constructible_v<T>
     struct roundrobin_pool
     {
     public:
@@ -85,7 +86,7 @@ namespace siddiqsoft
             // Increment counter *before* we invoke nextWorkerIndex..
             ++queueCounter;
             // Add into the thread's internal queue
-            workers.at(nextWorkerIndex()).queue(std::move(item));
+            workers.at(nextWorkerIndex()).queue(std::forward<T>(item));
         }
 
 #if defined(NLOHMANN_JSON_VERSION_MAJOR)

diff --git a/include/siddiqsoft/simple_pool.hpp b/include/siddiqsoft/simple_pool.hpp
@@ -39,6 +39,7 @@
 #include "simple_worker.hpp"
 #include <optional>
 #include <latch>
+#include "siddiqsoft/RunOnEnd.hpp"
 
 namespace siddiqsoft
 {
@@ -49,7 +50,7 @@ namespace siddiqsoft
     /// @remarks The number of threads in the pool is determined by the nature of your "work". If you're spending time against db
     /// then you might wish to use more threads as individual queries might take time and hog the thread.
     template <typename T, uint16_t N = 0>
-        requires std::move_constructible<T>
+        requires std::is_move_constructible_v<T>
     struct simple_pool
     {
         simple_pool(simple_pool&&)            = delete;
@@ -79,7 +80,7 @@ namespace siddiqsoft
         /// @brief Contructs a threadpool with N threads with the given callback/worker function
         /// @param c The worker function.
         simple_pool(std::function<void(T&&)> c)
-            : callback(c)
+            : callback(std::move(c))
         {
             // *CRITICAL*
             // This is step is *critical* otherwise we will end up moving threads as we add elements to the vector.
@@ -97,7 +98,7 @@ namespace siddiqsoft
                             // The getNextItem performs the wait on the signal and if it expires, returns empty.
                             // If there is an item, it will get that item (minimizing move) and performs the pop
                             // and returns the item so we can invoke the callback outside the lock.
-                            if (auto item = getNextItem(signalWaitInterval); item && !st.stop_requested()) {
+                            if (auto item = getNextItem(signalWaitInterval); item.has_value() && !st.stop_requested()) {
                                 // Delegate to the callback outside the lock
                                 callback(std::move(*item));
                             }
@@ -116,7 +117,7 @@ namespace siddiqsoft
             {
                 std::unique_lock<std::shared_mutex> myWriterLock(items_mutex);
 
-                items.emplace_back(std::move(item));
+                items.emplace_back(std::forward<T>(item));
             }
             signal.release();
             ++queueCounter;
@@ -165,13 +166,10 @@ namespace siddiqsoft
         {
             if (signal.try_acquire_for(signalWaitInterval)) {
                 // Guard against empty signals which are terminating indicator
-                std::unique_lock<std::shared_mutex> myWriterLock(items_mutex);
-                // Guard against empty signals which are terminating indicator
-                if (!items.empty()) {
+                if (std::unique_lock<std::shared_mutex> myWriterLock(items_mutex); !items.empty()) {
+                    siddiqsoft::RunOnEnd onCleanup([&]() { items.pop_front(); });
                     // WE require that the stored type by move-constructible!
-                    T item {std::move(items.front())};
-                    items.pop_front();
-                    return std::move(item);
+                    return std::move(items.front());
                 }
             }
 

diff --git a/include/siddiqsoft/simple_worker.hpp b/include/siddiqsoft/simple_worker.hpp
@@ -1,5 +1,5 @@
 /*
-    asynchrony-lib : Add asynchrony to your apps
+    asynchrony : Add asynchrony to your apps
 
     BSD 3-Clause License
 
@@ -46,6 +46,8 @@
 #include <semaphore>
 #include <stop_token>
 
+#include "siddiqsoft/RunOnEnd.hpp"
+
 
 namespace siddiqsoft
 {
@@ -177,13 +179,15 @@ namespace siddiqsoft
         std::optional<T> getNextItem(std::chrono::milliseconds& delta)
         {
             if (signal.try_acquire_for(signalWaitInterval)) {
-                std::unique_lock<std::shared_mutex> myWriterLock(items_mutex);
                 // Guard against empty signals which are terminating indicator
-                if (!items.empty()) {
+                if (std::unique_lock<std::shared_mutex> myWriterLock(items_mutex); !items.empty()) {
+                    RunOnEnd onScopeExit([&]() { items.pop_front(); });
                     // WE require that the stored type by move-constructible!
-                    T item {std::move(items.front())};
-                    items.pop_front();
-                    return std::move(item);
+                    return std::move(items.front());
+                    // The pop_front() happens on scope exit
+                    // T item {std::move(items.front())};
+                    // items.pop_front();
+                    // return std::move(item);
                 }
             }
 

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -10,22 +10,38 @@ if(${${PROJECT_NAME}_BUILD_TESTS})
                             PRIVATE
                             $<$<CXX_COMPILER_ID:Clang>:-fexperimental-library> )
 
+    if(CMAKE_CXX_COMPILER_ID MATCHES [Cc][Ll][Aa][Nn][Gg])
+        message(STATUS "Using sanitizers for leak and address on ${CMAKE_CXX_COMPILER_ID}")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address,leak")
+        target_link_libraries(${TESTPROJ} PRIVATE -fsanitize=address,leak)
+        message(STATUS "Enable instrumentation for code coverage")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-instr-generate -fcoverage-mapping")
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-instr-generate")
+        target_compile_options(${TESTPROJ} PRIVATE -fprofile-instr-generate -fcoverage-mapping)
+        target_link_options(${TESTPROJ} PRIVATE -fprofile-instr-generate) 
+    endif()
+
+    if( CMAKE_BUILD_TYPE MATCHES [Dd][Ee][Bb][Uu][Gg])
+        add_compile_definitions(${TESTPROJ} PRIVATE DEBUG=1)
+    endif()
+
     target_sources( ${TESTPROJ}
                     PRIVATE
                     ${PROJECT_SOURCE_DIR}/tests/periodic_worker.cpp
                     ${PROJECT_SOURCE_DIR}/tests/roundrobin_pool.cpp
                     ${PROJECT_SOURCE_DIR}/tests/simple_pool.cpp
                     ${PROJECT_SOURCE_DIR}/tests/simple_worker.cpp)
 
-    # Dependencies
+    # Dependencies (specifically and only for the tests program)
     cpmaddpackage("gh:google/googletest#v1.15.2")
     target_link_libraries(${TESTPROJ} PRIVATE GTest::gtest_main)
     cpmaddpackage("gh:nlohmann/json#v3.11.3")
     target_link_libraries(${TESTPROJ} PRIVATE nlohmann_json::nlohmann_json)
 
-
+    # Dependencies (specified by the library and to its clients)
+    target_link_libraries(${TESTPROJ} PRIVATE RunOnEnd::RunOnEnd)
+
     include(GoogleTest)
-
     gtest_discover_tests(${TESTPROJ} XML_OUTPUT_DIR "${PROJECT_SOURCE_DIR}/tests/results")
 
     include(CTest)