Skip to content
This repository was archived by the owner on Jul 31, 2023. It is now read-only.

Commit 1789eaf

Browse files
Emmanuel T OdekeRamon Nogueira
authored andcommitted
trace: reorder defaultIDGenerator fields for 8byte alignment (#866)
Fixes #865 This bug manifested as a consequence of https://golang.org/pkg/sync/atomic/#pkg-note-BUG and was exposed by PR #851 which switched to atomically incrementing defaultIDGenerator.nextSpanID The organization of the fields was misaligned on 32-bit machines because the field `traceIDRand *rand.Rand`, a pointer was included as the second field of the struct. This is because the size of a pointer on 32-bit machines is 4 bytes, hence after the second field, we'll have offset from 12 bytes and for atomic access of *int64 fields, which are accessed in 4 byte increments by atomic operations, on 32-bit machines, their addresses are on non-8-byte divisible alignments i.e. * nextSpanID -- [28, 36] * spanIDInc -- [36, 44] but on 64-bit machines, sizeof(pointer) = 8 bytes hence their addresses are on 8-byte divisible alignments i.e. * nextSpanID -- [32, 40] * spanIDInc -- [40, 48] Thus the required reorganization but making the pointer the last field fixes the problem for both 32-bit and 64-bit. This fix can be verified by prefixing `GOARCH=386` before running code or tests.
1 parent 457d67e commit 1789eaf

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

trace/trace.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,10 +470,20 @@ func init() {
470470

471471
type defaultIDGenerator struct {
472472
sync.Mutex
473-
traceIDRand *rand.Rand
473+
474+
// Please keep these as the first fields
475+
// so that these 8 byte fields will be aligned on addresses
476+
// divisible by 8, on both 32-bit and 64-bit machines when
477+
// performing atomic increments and accesses.
478+
// See:
479+
// * https://github.com/census-instrumentation/opencensus-go/issues/587
480+
// * https://github.com/census-instrumentation/opencensus-go/issues/865
481+
// * https://golang.org/pkg/sync/atomic/#pkg-note-BUG
482+
nextSpanID uint64
483+
spanIDInc uint64
484+
474485
traceIDAdd [2]uint64
475-
nextSpanID uint64
476-
spanIDInc uint64
486+
traceIDRand *rand.Rand
477487
}
478488

479489
// NewSpanID returns a non-zero span ID from a randomly-chosen sequence.

0 commit comments

Comments
 (0)