go/types: fix race-y initialization of Struct.offsets

Use sync.Once to ensure, that 'offsets' field is initialized once only in a threadsafe way. Fixes golang#12887 Change-Id: I90ef929c421ccd3094339c67a39b02d8f2e47211 Reviewed-on: https://go-review.googlesource.com/16013 Reviewed-by: Robert Griesemer <[email protected]>
gosuri · Dec 1, 2015 · a4cd0a4 · a4cd0a4
1 parent 5ff309f
commit a4cd0a4
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 26 deletions.
diff --git a/src/go/types/sizes.go b/src/go/types/sizes.go
@@ -132,13 +132,8 @@ func (s *StdSizes) Sizeof(T Type) int64 {
 		if n == 0 {
 			return 0
 		}
-		offsets := t.offsets
-		if t.offsets == nil {
-			// compute offsets on demand
-			offsets = s.Offsetsof(t.fields)
-			t.offsets = offsets
-		}
-		return offsets[n-1] + s.Sizeof(t.fields[n-1].typ)
+		setOffsets(t, s)
+		return t.offsets[n-1] + s.Sizeof(t.fields[n-1].typ)
 	case *Interface:
 		return s.WordSize * 2
 	}
@@ -159,24 +154,27 @@ func (conf *Config) alignof(T Type) int64 {
 }
 
 func (conf *Config) offsetsof(T *Struct) []int64 {
-	offsets := T.offsets
-	if offsets == nil && T.NumFields() > 0 {
+	var offsets []int64
+	if T.NumFields() > 0 {
 		// compute offsets on demand
 		if s := conf.Sizes; s != nil {
-			offsets = s.Offsetsof(T.fields)
-			// sanity checks
-			if len(offsets) != T.NumFields() {
-				panic("Config.Sizes.Offsetsof returned the wrong number of offsets")
-			}
-			for _, o := range offsets {
-				if o < 0 {
-					panic("Config.Sizes.Offsetsof returned an offset < 0")
+			calculated := setOffsets(T, s)
+			offsets = T.offsets
+			if calculated {
+				// sanity checks
+				if len(offsets) != T.NumFields() {
+					panic("Config.Sizes.Offsetsof returned the wrong number of offsets")
+				}
+				for _, o := range offsets {
+					if o < 0 {
+						panic("Config.Sizes.Offsetsof returned an offset < 0")
+					}
 				}
 			}
 		} else {
-			offsets = stdSizes.Offsetsof(T.fields)
+			setOffsets(T, &stdSizes)
+			offsets = T.offsets
 		}
-		T.offsets = offsets
 	}
 	return offsets
 }
@@ -209,3 +207,15 @@ func align(x, a int64) int64 {
 	y := x + a - 1
 	return y - y%a
 }
+
+// setOffsets sets the offsets of s for the given sizes if necessary.
+// The result is true if the offsets were not set before; otherwise it
+// is false.
+func setOffsets(s *Struct, sizes Sizes) bool {
+	var calculated bool
+	s.offsetsOnce.Do(func() {
+		calculated = true
+		s.offsets = sizes.Offsetsof(s.fields)
+	})
+	return calculated
+}
diff --git a/src/go/types/type.go b/src/go/types/type.go
@@ -4,9 +4,10 @@
 
 package types
 
-import "sort"
-
-// TODO(gri) Revisit factory functions - make sure they have all relevant parameters.
+import (
+	"sort"
+	"sync"
+)
 
 // A Type represents a type of Go.
 // All types implement the Type interface.
@@ -120,10 +121,10 @@ func (s *Slice) Elem() Type { return s.elem }
 
 // A Struct represents a struct type.
 type Struct struct {
-	fields []*Var
-	tags   []string // field tags; nil if there are no tags
-	// TODO(gri) access to offsets is not threadsafe - fix this
-	offsets []int64 // field offsets in bytes, lazily initialized
+	fields      []*Var
+	tags        []string  // field tags; nil if there are no tags
+	offsets     []int64   // field offsets in bytes, lazily initialized
+	offsetsOnce sync.Once // for threadsafe lazy initialization of offsets
 }
 
 // NewStruct returns a new struct with the given fields and corresponding field tags.