Browse Source

codec: fix potential data race where atomic.cas was used instead of once.Do

When looking up basicHandle from a handle, we also needed to do
some initialization.

We previously used a compare-and-swap, but that left open the
possibility of a data race during the one-time initialization.

Fix by using the equivalent of a once.Do.

Fixes #292
Ugorji Nwoke 6 years ago
parent
commit
28cf408da6
1 changed files with 18 additions and 4 deletions
  1. 18 4
      codec/helper.go

+ 18 - 4
codec/helper.go

@@ -582,10 +582,24 @@ type BasicHandle struct {
 // basicHandle returns an initialized BasicHandle from the Handle.
 func basicHandle(hh Handle) (x *BasicHandle) {
 	x = hh.getBasicHandle()
-	if atomic.CompareAndSwapUint32(&x.inited, 0, 1) {
-		x.be = hh.isBinary()
-		_, x.js = hh.(*JsonHandle)
-		x.n = hh.Name()[0]
+	// ** We need to simulate once.Do, to ensure no data race within the block.
+	// ** Consequently, below would not work.
+	// if atomic.CompareAndSwapUint32(&x.inited, 0, 1) {
+	// 	x.be = hh.isBinary()
+	// 	_, x.js = hh.(*JsonHandle)
+	// 	x.n = hh.Name()[0]
+	// }
+
+	// simulate once.Do using our own stored flag and mutex
+	if atomic.LoadUint32(&x.inited) == 0 {
+		x.mu.Lock()
+		if x.inited == 0 {
+			x.be = hh.isBinary()
+			_, x.js = hh.(*JsonHandle)
+			x.n = hh.Name()[0]
+			atomic.StoreUint32(&x.inited, 1)
+		}
+		x.mu.Unlock()
 	}
 	return
 }