bshoshany / thread-pool

BS::thread_pool: a fast, lightweight, and easy-to-use C++17 thread pool library
MIT License
2.21k stars 253 forks source link

Add task destroying to task processing. #129

Closed risa2000 closed 11 months ago

risa2000 commented 11 months ago

Fixes a single issue when the task queued to the pool contains a capture of shared ref. In the original code the processed task was held indefinitely, until the worker thread exited or processed another task, which was the only way to release the shared ref. (Would probably be the same for a functor.)

Style

Have you formatted your code using the .clang-format file attached to this project? Yes.

Testing

Have you tested the new code using the provided automated test program BS_thread_pool_test.cpp (preferably with the provided multi-compiler test script BS_thread_pool_test.ps1) and/or performed any other tests to ensure that the new code works correctly? Used BS_thread_pool_test.ps1 with clang++ and msvc.

If so, please provide information about the test system(s):

bshoshany commented 11 months ago

Hi @risa2000 and thanks for the PR! This was already suggested in #124, and has been implemented in the upcoming v4.0.0 release. The new release should be out within the next few weeks.

risa2000 commented 11 months ago

@bshoshany Hi there! I checked #124 and you may notice that there is a slight difference in how the task is destroyed. I have included it into "task execution" interval by explicitly calling destructor inside the section where the running task counter is incremented (and the global task lock is unlocked). I believe this is a cleaner solution. My first approach was the same as in #124 but this way the task destruction happens at the end of the while loop, out of the execution tracking and potentially keeping the lock unnecessarily.

bshoshany commented 11 months ago

I see. I think an even cleaner solution is to have task created inside its own code block, so it's destructed automatically at the end of the block. In v4.0.0 the worker function has actually been substantially rewritten, to allow for stuff like initialization functions and task priority, so I'll look into the best way to do this with the new worker.