| Bug #74846 | abort_loop is volatile, likely not what intended | ||
|---|---|---|---|
| Submitted: | 14 Nov 2014 0:47 | Modified: | 13 Feb 2015 14:46 |
| Reporter: | Stewart Smith (OCA) | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: Compiling | Severity: | S2 (Serious) |
| Version: | 5.7.5 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
| Tags: | PowerPC | ||
[14 Nov 2014 0:48]
Stewart Smith
fix abort_loop (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Contribution: volatile-abort_loop.patch (text/x-patch), 11.24 KiB.
[3 Dec 2014 8:23]
Umesh Shastry
Hello Stewart Smith, Thank you for the report and contribution. Thanks, Umesh
[13 Feb 2015 14:46]
Paul Dubois
Noted in 5.8.0 changelog. In mysqld.cc, the abort_loop variable was quantified with volatile, which on some platforms could result in changes not being seen immeidately in threads running on different cores. Thanks to Stewart Smith for the patch.
Description: This likely affects PowerPC (e.g. POWER8), ARM and other architectures - as well as the code actually being incorrect of course :) In mysqld.cc, there's "bool volatile abort_loop;" and all around the server there are places where this is checked. One thread my set abort_loop and other threads are expected to check this every so often. Of course, volatile does not put in memory barriers anywhere, it just insists that the compiler must do loads and stores. These loads and stores are only consistent as to the current thread of execution though due to the lack of memory barriers. So... as the code currently sits, abort_loop may be set and it could be a rather long time before other threads pick up on it - essentially, they're waiting for whenever that cache line is flushed out to other processors, which depending on load, could be NEVER! In reality, it *probably* all turns out okay... but it all depends on what's going on elsewhere in the system. How to repeat: examine source code. I think I may have seen the result of this while running some tests on POWER8. Unfortunately due to the nature of these things it's probably near impossible to reliably reproduce an issue. I've *possibly* seen this a few times in my testing... or at least possibly this issue. Suggested fix: Apply my patch! My patch is the safe, cross-platform approach. On POWER8, we could use the miso ("make it so") instruction after the store to provide a hint that the change should be flushed out.... but it's a hint, not a full solution.