Fix test timeout redux #161
Conversation
src/common.hh
Outdated
| std::vector<std::thread> threads_; | ||
| mutable std::mutex jobs_mutex_; | ||
| std::condition_variable cv_; | ||
| std::queue<JobT> jobs_; | ||
|
|
||
| bool started_; | ||
| std::atomic<bool> should_stop_; | ||
| std::atomic<bool> is_accepting_jobs_; | ||
|
|
||
| /// Multithreading |
There was a problem hiding this comment.
Optional
This comment is not very informative. I think it's telling me that these methods are expected be executed on the worker threads this pool owns (and not on the client/calling thread). The other useful information is that the caller of these methods is expected to lock the require mutex to guarantee thread safety and consistency. So maybe update this along those lines?
There was a problem hiding this comment.
I'll just remove the comment.
| @@ -60,24 +60,22 @@ struct ThreadPool final | |||
| ThreadPool(size_t n_threads, std::function<void(const std::string&)> err); | |||
| ~ThreadPool() noexcept; | |||
|
|
|||
| void start(); | |||
| void push_to_job_queue(JobT&& job); | |||
| void await_stop() noexcept; | |||
There was a problem hiding this comment.
Optional
Add a comment here to explain that calling this will wait/block until pending jobs are complete. Also, after this is complete, no new jobs can be submitted.
| @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. | |||
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | |||
| and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
|
|
|||
| ## Unreleased | |||
| ## [0.1.5](https://github.com/acquire-project/acquire-driver-zarr/compare/v0.1.3...v0.1.4) - 2023-11-20 | |||
There was a problem hiding this comment.
I assume you're planning to tag the commit associated with merging this PR with v0.1.5? If so, looks good.
ThreadPoolin favor of start on construction.