Bug #74854 rpl_gtid Checkable_rwlock debug code doesn't need volatile lock_state
Submitted: 14 Nov 2014 6:19 Modified: 11 May 2015 16:38
Reporter: Stewart Smith (OCA) Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Replication Severity:S3 (Non-critical)
Version:5.7.7 OS:Any
Assigned to: CPU Architecture:Any

[14 Nov 2014 6:19] Stewart Smith
Description:
volatile gives you nothing over using proper atomics, the keyword is unneeded.

What *should* be there though is the atomic store of the initial value. This could be an actual bug on debug builds on POWER and ARM and similar.

This patch fixes the issue (attached):
Index: mysql-5.7.5-m15/sql/rpl_gtid.h
===================================================================
--- mysql-5.7.5-m15.orig/sql/rpl_gtid.h
+++ mysql-5.7.5-m15/sql/rpl_gtid.h
@@ -329,7 +329,7 @@ public:
   Checkable_rwlock()
   {
 #ifndef DBUG_OFF
-    lock_state= 0;
+    my_atomic_store32(&lock_state, 0);
 #else
     is_write_lock= false;
 #endif
@@ -423,7 +423,7 @@ private:
     -1 - write locked
     >0 - read locked by that many threads
   */
-  volatile int32 lock_state;
+  int32 lock_state;
   /// Read lock_state atomically and return the value.
   inline int32 get_state() const
   {

How to repeat:
code analysis

Suggested fix:
apply my patch!
[14 Nov 2014 6:20] Stewart Smith
fix debug code for rpl_gtid Checkable_rwlock() for POWER and ARM

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

Contribution: volatile-rpl-gtid.patch (text/x-patch), 620 bytes.

[17 Nov 2014 9:02] Umesh Shastry
Hello Stewart,

Thank you for the report and contribution.

Thanks,
Umesh
[10 Apr 2015 8:09] Stewart Smith
Still present in 5.7.7
[11 May 2015 16:38] David Moss
Thanks for your feedback. This has been fixed in an upcoming version.