pingcap/tidb

ddl/ddl.go: Recv from chan `task.err` may block forever when send is skipped

Open

#26,763 opened on Jul 31, 2021

View on GitHub
 (4 comments) (0 reactions) (1 assignee)Go (40,090 stars) (6,186 forks)batch import
help wantedsig/sql-infratype/enhancement

Description

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

We found this bug through a WIP Fuzzing project by system-pclub, PSU

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

In ddl_worker.go, limitDDLJobs() calls addBatchDDLJobs() on L272 if selecting the first case, which in turn calls sending operation task.err <-err on L311. https://github.com/pingcap/tidb/blob/fa71a0e46d427e8a7450a2dce27f8c0d4c02a45c/ddl/ddl_worker.go#L258-L275 https://github.com/pingcap/tidb/blob/fa71a0e46d427e8a7450a2dce27f8c0d4c02a45c/ddl/ddl_worker.go#L310-L311 Then in ddl.go, doDDLJob() calls receiving operation err := <-task.err on L540 https://github.com/pingcap/tidb/blob/fa71a0e46d427e8a7450a2dce27f8c0d4c02a45c/ddl/ddl.go#L534-L540

3. What did you see instead (Required)

If the select in limitDDLJobs() chooses the second case, the sending operation in function addBatchDDLJobs() will be skipped. Thus the receiving operation in doDDLJob() will block forever.

4. What is your TiDB version? (Required)

branch: master commit: 7e6690df8e8d5474b1872edbd279bb1b3c510ee5

Contributor guide

Tech stack
go
Domain
database
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Go programmingconcurrent programming with channelsTiDB DDL internals
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
60
Research direction
The bug is in ddl/ddl worker.go: the send on task.err is only done inside the first case of a select, so if the second case is chosen, the send is skipped. This causes ddl/ddl.go: the receive on task.err to block forever. A fix would be to ensure the send always happens, perhaps by using a buffered channel or moving the send outside the select. No linked PRs or comments suggest a solution.