Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MPPTask is not move to cancel thread succesfully in MPPTaskManager::cancelMPPQuery #5630

Closed
windtalker opened this issue Aug 16, 2022 · 1 comment
Labels
affects-6.2 severity/major type/bug The issue is confirmed as a bug.

Comments

@windtalker
Copy link
Contributor

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

In #5361, we try to move current_task to cancel thread in MPPTaskManager::cancelMPPQuery, so MPPTask can be destruct parallelly:

thread_manager->schedule(false, "CancelMPPTask", [task = std::move(current_task), &reason] { task->cancel(reason); });

However, it is found that due to llvm's SOO, current_task is actually not moved to cancel thread, in some corner cases, current_task will be destructed inside the loop:

for (auto it = task_set->task_map.begin(); it != task_set->task_map.end();)
{
fmt_buf.fmtAppend("{} ", it->first.toString());
auto current_task = it->second;
it = task_set->task_map.erase(it);
thread_manager->schedule(false, "CancelMPPTask", [task = std::move(current_task), &reason] { task->cancel(reason); });
}

It is very dangerous to deconstruct mpp task inside the loop since destruct a mpp task before all other mpp tasks are cancelled may cause some deadlock issues.

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your TiFlash version? (Required)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.2 severity/major type/bug The issue is confirmed as a bug.
Projects
None yet
Development

No branches or pull requests

2 participants