Bug #74833 log_state declared volatile, missing memory barriers

Description:
This will affect POWER, ARM and maybe others (depends on memory model).
See https://www.flamingspork.com/blog/2014/11/13/volatile-considered-harmful/ for further information

in sql/binlog.h:
class MYSQL_BIN_LOG: public TC_LOG
{
...
  volatile enum_log_state log_state;

This means that for accesses to log_state, load/store instructions will be generated but what will *not* be generated is any memory barriers. This means we will quite likely be reading stale log_state values on other cores, leading to all sorts of potential problems (MYSQL_BIN_LOG::is_open() which returns log_state is called from all over the place).

Not only that, but is_open() is declared as const... which is pretty much certainly completely and totally wrong.

How to repeat:
examine source code OR design some awful test that hits this... in reality, it's probably relatively tricky to hit it but I'm not sure you'd ever trace a random crash or misbehaviour to this issue.

Suggested fix:
Apply my patch (attached)!

My patch implements what was likely intended... that log_state is atomic, and the load/store instructions *actually* load and store the value.

convert log_state to atomic, generating correct load/store and barriers

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: volatile-binlog-log_state.patch (text/x-patch), 4.09 KiB.

Hello Stewart,

Thank you for the report and contribution.

Thanks,
Umesh

Still present in 5.7.7

Thanks for your feedback. This has been fixed in an upcoming version.