| Bug #72544 | Incorrect locking for global_query_id | ||
|---|---|---|---|
| Submitted: | 6 May 2014 7:18 | Modified: | 6 May 2014 7:22 |
| Reporter: | Dmitry Lenev | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: Stored Routines | Severity: | S3 (Non-critical) |
| Version: | 5.6.17 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[6 May 2014 15:39]
Mark Callaghan
Asking because I don't understand. Why is this incorrect?
[6 May 2014 16:13]
Dmitry Lenev
Hello Mark! At the very least locking and unlocking of LOCK_thread_count in this situation is redundant with the current 5.5/5.6 code. Plus, at least in theory, on some platforms such read can be non-atomic/we can observe funny memory visibility issues. It is better to be on the safe side and follow the protocol (or document why we don't follow it).
[28 May 2014 4:53]
Stewart Smith
Yeah, in 5.6 global_query_id is just an atomic variable, so should just use the atomics builtins for accessing it. The mutex lock isn't needed here and may block other things, although it's possible that the memory barriers in the mutex implementation end up having the resultant code be "correct" by simply locking too much.
Description: Reporting on behalf of Stewart Smith, who has originally reported this issue on internals@ list. In sql/sp_head.cc: bool sp_head::execute_function(THD *thd, Item **argp, uint argcount, We have this code: mysql_mutex_lock(&LOCK_thread_count); q= global_query_id; mysql_mutex_unlock(&LOCK_thread_count); Which is incorrect. It should at least by a my_atomic_load64 with the appropriate lock things around it. I haven't looked at if the mysql_bin.log.start_union_events(thd, q+1) is correct or not. How to repeat: Inspect the code. Suggested fix: See above.