Skip to content

Commit 5ce50fe

Browse files
committed
Add explicit creation methods for task manager and runners
Particularly for the task manager, this ensures these objects are held by a shared pointer and properly initialized.
1 parent bdf0d5e commit 5ce50fe

17 files changed

+215
-115
lines changed

fly/task/task_manager.cpp

+19-3
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,25 @@ namespace {
1414
} // namespace
1515

1616
//==================================================================================================
17-
TaskManager::TaskManager(std::uint32_t num_workers) noexcept :
17+
std::shared_ptr<TaskManager> TaskManager::create(std::uint32_t thread_count)
18+
{
19+
// TaskManager has a private constructor, thus cannot be used with std::make_shared. This class
20+
// is used to expose the private constructor locally.
21+
struct TaskManagerImpl final : public TaskManager
22+
{
23+
explicit TaskManagerImpl(std::uint32_t thread_count) noexcept : TaskManager(thread_count)
24+
{
25+
}
26+
};
27+
28+
auto task_manager = std::make_shared<TaskManagerImpl>(thread_count);
29+
return task_manager->start() ? task_manager : nullptr;
30+
}
31+
32+
//==================================================================================================
33+
TaskManager::TaskManager(std::uint32_t thread_count) noexcept :
1834
m_keep_running(false),
19-
m_num_workers(num_workers)
35+
m_thread_count(thread_count)
2036
{
2137
}
2238

@@ -29,7 +45,7 @@ bool TaskManager::start()
2945
{
3046
std::shared_ptr<TaskManager> task_manager = shared_from_this();
3147

32-
for (std::uint32_t i = 0; i < m_num_workers; ++i)
48+
for (std::uint32_t i = 0; i < m_thread_count; ++i)
3349
{
3450
m_futures.push_back(
3551
std::async(std::launch::async, &TaskManager::worker_thread, task_manager));

fly/task/task_manager.hpp

+13-24
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,13 @@ class TaskManager : public std::enable_shared_from_this<TaskManager>
3232

3333
public:
3434
/**
35-
* Constructor.
35+
* Create and start a task manager.
36+
*
37+
* @param thread_count Size of the thread pool for the task manager to own.
3638
*
37-
* @param num_workers Number of worker threads to create.
39+
* @return The created task manager.
3840
*/
39-
explicit TaskManager(std::uint32_t num_workers) noexcept;
41+
static std::shared_ptr<TaskManager> create(std::uint32_t thread_count);
4042

4143
/**
4244
* Create the worker threads and timer thread.
@@ -58,16 +60,6 @@ class TaskManager : public std::enable_shared_from_this<TaskManager>
5860
*/
5961
bool stop();
6062

61-
/**
62-
* Create a task runner, holding a weak reference to this task manager.
63-
*
64-
* @tparam TaskRunnerType The type of task runner to create.
65-
*
66-
* @return The created task runner.
67-
*/
68-
template <typename TaskRunnerType>
69-
std::shared_ptr<TaskRunnerType> create_task_runner();
70-
7163
private:
7264
/**
7365
* Wrapper structure to associate a task with its task runner and the point in time that the
@@ -81,6 +73,13 @@ class TaskManager : public std::enable_shared_from_this<TaskManager>
8173
std::chrono::steady_clock::time_point m_schedule;
8274
};
8375

76+
/**
77+
* Constructor.
78+
*
79+
* @param thread_count Size of the thread pool for the task manager to own.
80+
*/
81+
explicit TaskManager(std::uint32_t thread_count) noexcept;
82+
8483
/**
8584
* Post a task to be executed as soon as a worker thread is available.
8685
*
@@ -124,17 +123,7 @@ class TaskManager : public std::enable_shared_from_this<TaskManager>
124123

125124
std::vector<std::future<void>> m_futures;
126125

127-
std::uint32_t m_num_workers;
126+
std::uint32_t m_thread_count;
128127
};
129128

130-
//==================================================================================================
131-
template <typename TaskRunnerType>
132-
std::shared_ptr<TaskRunnerType> TaskManager::create_task_runner()
133-
{
134-
static_assert(std::is_base_of_v<TaskRunner, TaskRunnerType>);
135-
136-
const std::shared_ptr<TaskManager> task_manager = shared_from_this();
137-
return std::shared_ptr<TaskRunnerType>(new TaskRunnerType(task_manager));
138-
}
139-
140129
} // namespace fly

fly/task/task_runner.cpp

+42-6
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
namespace fly {
66

77
//==================================================================================================
8-
TaskRunner::TaskRunner(std::weak_ptr<TaskManager> weak_task_manager) noexcept :
9-
m_weak_task_manager(std::move(weak_task_manager))
8+
TaskRunner::TaskRunner(const std::shared_ptr<TaskManager> &task_manager) noexcept :
9+
m_weak_task_manager(task_manager)
1010
{
1111
}
1212

@@ -46,8 +46,25 @@ void TaskRunner::execute(TaskLocation &&location, Task &&task)
4646
}
4747

4848
//==================================================================================================
49-
ParallelTaskRunner::ParallelTaskRunner(std::weak_ptr<TaskManager> weak_task_manager) noexcept :
50-
TaskRunner(std::move(weak_task_manager))
49+
std::shared_ptr<ParallelTaskRunner>
50+
ParallelTaskRunner::create(const std::shared_ptr<TaskManager> &task_manager)
51+
{
52+
// ParallelTaskRunner has a private constructor, thus cannot be used with std::make_shared. This
53+
// class is used to expose the private constructor locally.
54+
struct ParallelTaskRunnerImpl final : public ParallelTaskRunner
55+
{
56+
explicit ParallelTaskRunnerImpl(const std::shared_ptr<TaskManager> &task_manager) noexcept :
57+
ParallelTaskRunner(task_manager)
58+
{
59+
}
60+
};
61+
62+
return std::make_shared<ParallelTaskRunnerImpl>(task_manager);
63+
}
64+
65+
//==================================================================================================
66+
ParallelTaskRunner::ParallelTaskRunner(const std::shared_ptr<TaskManager> &task_manager) noexcept :
67+
TaskRunner(task_manager)
5168
{
5269
}
5370

@@ -63,8 +80,27 @@ void ParallelTaskRunner::task_complete(TaskLocation &&)
6380
}
6481

6582
//==================================================================================================
66-
SequencedTaskRunner::SequencedTaskRunner(std::weak_ptr<TaskManager> weak_task_manager) noexcept :
67-
TaskRunner(std::move(weak_task_manager))
83+
std::shared_ptr<SequencedTaskRunner>
84+
SequencedTaskRunner::create(const std::shared_ptr<TaskManager> &task_manager)
85+
{
86+
// SequencedTaskRunner has a private constructor, thus cannot be used with std::make_shared.
87+
// This class is used to expose the private constructor locally.
88+
struct SequencedTaskRunnerImpl final : public SequencedTaskRunner
89+
{
90+
explicit SequencedTaskRunnerImpl(const std::shared_ptr<TaskManager> &task_manager) noexcept
91+
:
92+
SequencedTaskRunner(task_manager)
93+
{
94+
}
95+
};
96+
97+
return std::make_shared<SequencedTaskRunnerImpl>(task_manager);
98+
}
99+
100+
//==================================================================================================
101+
SequencedTaskRunner::SequencedTaskRunner(const std::shared_ptr<TaskManager> &task_manager) noexcept
102+
:
103+
TaskRunner(task_manager)
68104
{
69105
}
70106

fly/task/task_runner.hpp

+24-6
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ class TaskRunner : public std::enable_shared_from_this<TaskRunner>
296296
/**
297297
* Private constructor. Task runners may only be created by the task manager.
298298
*
299-
* @param weak_task_manager The task manager.
299+
* @param task_manager The task manager.
300300
*/
301-
TaskRunner(std::weak_ptr<TaskManager> weak_task_manager) noexcept;
301+
TaskRunner(const std::shared_ptr<TaskManager> &task_manager) noexcept;
302302

303303
/**
304304
* Post a task for execution in accordance with the concrete task runner's policy.
@@ -440,10 +440,19 @@ class TaskRunner : public std::enable_shared_from_this<TaskRunner>
440440
*/
441441
class ParallelTaskRunner : public TaskRunner
442442
{
443-
friend class TaskManager;
443+
public:
444+
/**
445+
* Create a parallel task runner.
446+
*
447+
* @param task_manager The task manager this runner should interface with.
448+
*
449+
* @return The created task runner.
450+
*/
451+
static std::shared_ptr<ParallelTaskRunner>
452+
create(const std::shared_ptr<TaskManager> &task_manager);
444453

445454
protected:
446-
explicit ParallelTaskRunner(std::weak_ptr<TaskManager>) noexcept;
455+
explicit ParallelTaskRunner(const std::shared_ptr<TaskManager> &task_manager) noexcept;
447456

448457
/**
449458
* Post a task for execution immediately.
@@ -477,10 +486,19 @@ class ParallelTaskRunner : public TaskRunner
477486
*/
478487
class SequencedTaskRunner : public TaskRunner
479488
{
480-
friend class TaskManager;
489+
public:
490+
/**
491+
* Create a sequenced task runner.
492+
*
493+
* @param task_manager The task manager this runner should interface with.
494+
*
495+
* @return The created task runner.
496+
*/
497+
static std::shared_ptr<SequencedTaskRunner>
498+
create(const std::shared_ptr<TaskManager> &task_manager);
481499

482500
protected:
483-
explicit SequencedTaskRunner(std::weak_ptr<TaskManager>) noexcept;
501+
explicit SequencedTaskRunner(const std::shared_ptr<TaskManager> &task_manager) noexcept;
484502

485503
/**
486504
* Post a task for execution within this sequence. If a task is not already running, the task is

test/config/config_manager.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ class BadConfig : public fly::Config
4848

4949
CATCH_TEST_CASE("ConfigManager", "[config]")
5050
{
51-
auto task_runner =
52-
fly::test::task_manager()->create_task_runner<fly::test::WaitableSequencedTaskRunner>();
51+
auto task_runner = fly::test::WaitableSequencedTaskRunner::create(fly::test::task_manager());
5352

5453
fly::test::PathUtil::ScopedTempDirectory config_path;
5554
std::filesystem::path config_file = config_path.file();

test/logger/logger.cpp

+1-5
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,7 @@ CATCH_TEST_CASE("Logger", "[logger]")
194194
// Run all of the log point tests with both synchronous and asynchronous loggers.
195195
const bool synchronous_logger = GENERATE(true, false);
196196

197-
auto task_manager = std::make_shared<fly::TaskManager>(1);
198-
CATCH_REQUIRE(task_manager->start());
199-
200-
auto task_runner =
201-
fly::test::task_manager()->create_task_runner<fly::SequencedTaskRunner>();
197+
auto task_runner = fly::SequencedTaskRunner::create(fly::test::task_manager());
202198
auto sink = std::make_unique<QueueSink>(received_logs);
203199

204200
auto logger = synchronous_logger ?

test/net/listen_socket.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ CATCH_TEMPLATE_TEST_CASE("AsyncListenSocket", "[net]", fly::net::IPv4Address, fl
297297
using ListenSocket = fly::net::ListenSocket<EndpointType>;
298298
using TcpSocket = fly::net::TcpSocket<EndpointType>;
299299

300-
auto task_runner = fly::test::task_manager()->create_task_runner<fly::SequencedTaskRunner>();
300+
auto task_runner = fly::SequencedTaskRunner::create(fly::test::task_manager());
301301
auto socket_service = fly::net::SocketService::create(task_runner);
302302
fly::test::Signal signal;
303303

test/net/socket_service.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ CATCH_TEMPLATE_TEST_CASE("SocketService", "[net]", fly::net::IPv4Address, fly::n
3535
using EndpointType = fly::net::Endpoint<IPAddressType>;
3636
using UdpSocket = fly::net::UdpSocket<EndpointType>;
3737

38-
auto task_runner = fly::test::task_manager()->create_task_runner<fly::SequencedTaskRunner>();
38+
auto task_runner = fly::SequencedTaskRunner::create(fly::test::task_manager());
3939
auto socket_service = fly::net::SocketService::create(task_runner);
4040
fly::test::Signal signal;
4141

test/net/tcp_socket.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ CATCH_TEMPLATE_TEST_CASE("AsyncTcpSocket", "[net]", fly::net::IPv4Address, fly::
321321
using ListenSocket = fly::net::ListenSocket<EndpointType>;
322322
using TcpSocket = fly::net::TcpSocket<EndpointType>;
323323

324-
auto task_runner = fly::test::task_manager()->create_task_runner<fly::SequencedTaskRunner>();
324+
auto task_runner = fly::SequencedTaskRunner::create(fly::test::task_manager());
325325
auto socket_service = fly::net::SocketService::create(task_runner);
326326

327327
const std::string message(fly::String::generate_random_string(1 << 10));

test/net/udp_socket.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ CATCH_TEMPLATE_TEST_CASE("AsyncUdpSocket", "[net]", fly::net::IPv4Address, fly::
257257
using EndpointType = fly::net::Endpoint<IPAddressType>;
258258
using UdpSocket = fly::net::UdpSocket<EndpointType>;
259259

260-
auto task_runner = fly::test::task_manager()->create_task_runner<fly::SequencedTaskRunner>();
260+
auto task_runner = fly::SequencedTaskRunner::create(fly::test::task_manager());
261261
auto socket_service = fly::net::SocketService::create(task_runner);
262262

263263
const std::string message(fly::String::generate_random_string(1 << 10));

test/path/path_monitor.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ class TestPathConfig : public fly::PathConfig
4646

4747
CATCH_TEST_CASE("PathMonitor", "[path]")
4848
{
49-
auto task_runner =
50-
fly::test::task_manager()->create_task_runner<fly::test::WaitableSequencedTaskRunner>();
49+
auto task_runner = fly::test::WaitableSequencedTaskRunner::create(fly::test::task_manager());
5150

5251
auto monitor =
5352
std::make_shared<fly::PathMonitorImpl>(task_runner, std::make_shared<TestPathConfig>());

test/system/system_monitor.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ class TestSystemConfig : public fly::SystemConfig
4343

4444
CATCH_TEST_CASE("SystemMonitor", "[system]")
4545
{
46-
auto task_runner =
47-
fly::test::task_manager()->create_task_runner<fly::test::WaitableSequencedTaskRunner>();
46+
auto task_runner = fly::test::WaitableSequencedTaskRunner::create(fly::test::task_manager());
4847

4948
auto monitor =
5049
std::make_shared<fly::SystemMonitorImpl>(task_runner, std::make_shared<TestSystemConfig>());

0 commit comments

Comments
 (0)