Skip to content

Commit dc16770

Browse files
committed
refactor(thread): improve task registration safety
* Use `QPointer` for safer task references in signal handlers, preventing potential access to deleted objects. * Store and remove tasks using precomputed IDs to avoid duplicated lookups and ensure consistency. * Update method signatures for clarity and type safety.
1 parent 60f04a8 commit dc16770

2 files changed

Lines changed: 16 additions & 13 deletions

File tree

src/core/thread/Task.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ Task::~Task() = default;
192192
*
193193
* @return QString
194194
*/
195-
QString Task::GetFullID() const { return p_->GetFullID(); }
195+
auto Task::GetFullID() const -> QString { return p_->GetFullID(); }
196196

197197
QString Task::GetUUID() const { return p_->GetUUID(); }
198198

src/core/thread/TaskRunner.cpp

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,23 @@ class TaskRunner::Impl : public QThread {
4848
task->SafelyRun();
4949
}
5050

51-
auto RegisterTask(const QString& name, const Task::TaskRunnable& runnerable,
52-
const Task::TaskCallback& cb,
53-
DataObjectPtr params) -> Task::TaskHandler {
54-
auto* raw_task = new Task(runnerable, name, std::move(params), cb);
51+
auto RegisterTask(const QString& name, const Task::TaskRunnable& runnable,
52+
const Task::TaskCallback& cb, DataObjectPtr params)
53+
-> Task::TaskHandler {
54+
auto* raw_task = new Task(runnable, name, std::move(params), cb);
5555
raw_task->setParent(nullptr);
5656
raw_task->moveToThread(this);
5757

58-
connect(raw_task, &Task::SignalRun, this, [this, raw_task]() {
59-
pending_tasks_[raw_task->GetFullID()] = raw_task;
60-
});
58+
const auto full_id = raw_task->GetFullID();
6159

62-
connect(raw_task, &Task::SignalTaskEnd, this, [this, raw_task]() {
63-
pending_tasks_.remove(raw_task->GetFullID());
64-
});
60+
connect(raw_task, &Task::SignalRun, this,
61+
[this, task = QPointer<Task>(raw_task), full_id]() {
62+
if (task == nullptr) return;
63+
pending_tasks_[full_id] = task;
64+
});
65+
66+
connect(raw_task, &Task::SignalTaskEnd, this,
67+
[this, full_id]() { pending_tasks_.remove(full_id); });
6568

6669
return Task::TaskHandler(raw_task);
6770
}
@@ -136,8 +139,8 @@ auto TaskRunner::IsRunning() -> bool { return p_->isRunning(); }
136139

137140
auto TaskRunner::RegisterTask(const QString& name,
138141
const Task::TaskRunnable& runnable,
139-
const Task::TaskCallback& cb,
140-
DataObjectPtr p_pbj) -> Task::TaskHandler {
142+
const Task::TaskCallback& cb, DataObjectPtr p_pbj)
143+
-> Task::TaskHandler {
141144
return p_->RegisterTask(name, runnable, cb, p_pbj);
142145
}
143146
} // namespace GpgFrontend::Thread

0 commit comments

Comments
 (0)