Skip to content

Commit

Permalink
完善线程池相关接口
Browse files Browse the repository at this point in the history
  • Loading branch information
@xia-chu
xia-chu committed Jan 5, 2025
1 parent 0421201 commit 8df4a64
Showing 1 changed file with 19 additions and 7 deletions.
26 changes: 19 additions & 7 deletions src/Thread/ThreadPool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ class ThreadPool : public TaskExecutor {
return nullptr;
}
auto ret = std::make_shared<Task>(std::move(task));
_queue.push_task(ret);
_queue.push_task([ret](size_t) {
(*ret)();
});
return ret;
}

Expand All @@ -70,10 +72,20 @@ class ThreadPool : public TaskExecutor {
}

auto ret = std::make_shared<Task>(std::move(task));
_queue.push_task_first(ret);
_queue.push_task_first([ret](size_t) {
(*ret)();
});
return ret;
}

void async2(std::function<void(size_t index)> task, bool may_sync = true) {
if (may_sync && _thread_group.is_this_thread_in()) {
task(0);
return;
}
_queue.push_task(std::move(task));
}

size_t size() {
return _queue.size();
}
Expand Down Expand Up @@ -119,17 +131,17 @@ class ThreadPool : public TaskExecutor {
private:
void run(size_t index) {
_on_setup(index);

Check warning on line 133 in src/Thread/ThreadPool.h

View workflow job for this annotation

GitHub Actions / build 

 'argument': conversion from 'size_t' to 'int', possible loss of data

Check warning on line 133 in src/Thread/ThreadPool.h

View workflow job for this annotation

GitHub Actions / build 

 'argument': conversion from 'size_t' to 'int', possible loss of data

Check warning on line 133 in src/Thread/ThreadPool.h

View workflow job for this annotation

GitHub Actions / build 

 'argument': conversion from 'size_t' to 'int', possible loss of data

Check warning on line 133 in src/Thread/ThreadPool.h

View workflow job for this annotation

GitHub Actions / build 

 'argument': conversion from 'size_t' to 'int', possible loss of data

Check warning on line 133 in src/Thread/ThreadPool.h

View workflow job for this annotation

GitHub Actions / build 

 'argument': conversion from 'size_t' to 'int', possible loss of data

Check warning on line 133 in src/Thread/ThreadPool.h

View workflow job for this annotation

GitHub Actions / build 

 'argument': conversion from 'size_t' to 'int', possible loss of data
Task::Ptr task;
std::function<void(size_t index)> task;
while (true) {
startSleep();
if (!_queue.get_task(task)) {
//空任务，退出线程 [AUTO-TRANSLATED:583e2f11]
//Empty task, exit the thread
// 空任务，退出线程 [AUTO-TRANSLATED:583e2f11]
// Empty task, exit the thread
break;
}
sleepWakeUp();
try {
(*task)();
task(index);
task = nullptr;
} catch (std::exception &ex) {
ErrorL << "ThreadPool catch a exception: " << ex.what();
Expand All @@ -149,7 +161,7 @@ class ThreadPool : public TaskExecutor {
size_t _thread_num;
Logger::Ptr _logger;
thread_group _thread_group;
TaskQueue<Task::Ptr> _queue;
TaskQueue<std::function<void(size_t index)>> _queue;
std::function<void(int)> _on_setup;
};

Expand Down

2 comments on commit 8df4a64

@alexliyu7352
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect the thread pool related interfaces

TRANS_BY_GITHUB_AI_ASSISTANT

@alexliyu7352
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review for src/Thread/ThreadPool.h:

Code Review: Patch to ThreadPool.h

Summary

This patch modifies the ThreadPool class to improve its task handling and adds a new async2 method. It's a patch review focusing on the changes introduced.

Detailed Feedback

Code Overview

The patch refactors the async and async_first methods to use lambdas to wrap the tasks before pushing them to the task queue. This allows the task execution to be handled consistently within the worker threads, receiving the thread index. Additionally, a new async2 method is added, providing a more flexible way to submit tasks that directly accept a thread index. The internal task queue type is changed to accommodate this.

Strengths

  • Improved Task Handling: The use of lambdas ensures that the task execution is handled correctly within the worker threads, even if the original task doesn't explicitly handle thread index. This improves robustness and consistency.
  • Added Flexibility: The async2 method provides a more flexible way to submit tasks, especially useful when the task logic needs the thread index.
  • Type Safety: Changing the TaskQueue type to TaskQueue<std::function<void(size_t index)>> improves type safety and clarifies the expected task signature.
  • Clearer Comments: The comments are improved, especially the translation notes.

Areas for Improvement

1. Error Handling in async2

  • Issue: The async2 method doesn't handle potential exceptions thrown by the task. If a task within async2 throws an exception, it will likely crash the thread.
  • Suggestion: Wrap the task execution in a try-catch block similar to what's done in the run method. Log the exception and potentially take other recovery actions.
  • Example:
    void async2(std::function<void(size_t index)> task, bool may_sync = true) {
        if (may_sync && _thread_group.is_this_thread_in()) {
            task(0);
            return;
        }
        _queue.push_task([task, this](size_t index) {
            try {
                task(index);
            } catch (const std::exception& ex) {
                ErrorL << "ThreadPool catch a exception in async2: " << ex.what();
                // Consider adding more robust error handling here, e.g., retry mechanism or task queue removal
            }
        });
    }

2. Potential for Deadlock

  • Issue: While unlikely, there's a small potential for deadlock if _queue.push_exit(_thread_num) in shutdown is called while threads are blocked in _queue.get_task(task). If the queue is empty and no tasks are added after push_exit, the threads might wait indefinitely.
  • Suggestion: Consider adding a timeout to _queue.get_task(task) or a mechanism to signal threads to exit gracefully even if the queue is empty. A condition variable could be used to coordinate shutdown.

3. Documentation

  • Issue: The new async2 method lacks documentation.
  • Suggestion: Add a Doxygen-style comment explaining its purpose, parameters, and return value (void).

Conclusion

The patch is a good improvement to the ThreadPool class, enhancing its flexibility and robustness. Addressing the error handling in async2 and considering potential deadlock scenarios are crucial for improving its reliability. Adding documentation for async2 will improve code clarity. The changes are generally well-implemented and follow good coding practices.

TRANS_BY_GITHUB_AI_ASSISTANT

Please sign in to comment.