Bug #73633 | extra mysql_mutex_lock in pop_jobs_item | ||
---|---|---|---|
Submitted: | 19 Aug 2014 7:18 | Modified: | 7 Aug 2020 8:51 |
Reporter: | qinglin zhang (OCA) | Email Updates: | |
Status: | Verified | Impact on me: | |
Category: | MySQL Server: Replication | Severity: | S3 (Non-critical) |
Version: | 5.6, 5.7 | OS: | Any |
Assigned to: | CPU Architecture: | Any | |
Tags: | extra mysql_mutex_lock in pop_jobs_item |
[19 Aug 2014 7:18]
qinglin zhang
[7 Aug 2020 8:51]
MySQL Verification Team
Hi, thanks for the report. I tend to agree with your assessment of the code so verifying this. Let's see what the replication team will say. all best Bogdan
[12 Aug 2020 19:54]
Sven Sandberg
Posted by developer: In 5.6, thd->EXIT_COND will unlock the mutex. So the mutex is not held at the time of the lock. The logic is more explicit in 5.7+8.0, where EXIT_COND was changed so it does not unlock the mutex. To compensate for that change, an explicit unlock was added in pop_jobs_item. So everything works as expected. However, this code (and several other places in rpl_rli_pdb.cc) follows the wrong pattern. The correct way to wait on a condition variable in MySQL is: PSI_stage_info old_stage; mysql_mutex_lock(&mutex); thd->ENTER_COND(&cond, &lock, &stage, &old_stage); while (<predicate> && !thd->killed) mysql_cond_wait(&cond, &mutex); mysql_mutex_unlock(&mutex); thd->EXIT_COND(&old_stage); The code in rpl_rli_pdb.cc has two disadvantages: - It does an extra unlock/lock inside the loop. On the normal execution path where it only makes one iteration of the loop, the result is one extra (very brief) lock/unlock. - It briefly switches to the old thread stage in case of spurious wakeups. This might be observable, and potentially misleading, for a user that checks the thread stage in performance_schema.threads or processlist. So the bug report is valid and the correct fix in 5.7+8.0 would be to move ENTER_COND before the loop, move EXIT_COND after the unlock that is done after the loop, and remove the unlock/lock pair from the loop.