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

Commit

Permalink
trace: reorder defaultIDGenerator fields for 8byte alignment
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
odeke-em committed Aug 16, 2018
1 parent 457d67e commit 67abf0e
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,20 @@ func init() {

type defaultIDGenerator struct {
sync.Mutex
traceIDRand *rand.Rand

// Please keep these as the first fields
// so that these 8 byte fields will be aligned on addresses
// divisible by 8, on both 32-bit and 64-bit machines when
// performing atomic increments and accesses.
// See:
// * https://github.com/census-instrumentation/opencensus-go/issues/587
// * https://github.com/census-instrumentation/opencensus-go/issues/865
// * https://golang.org/pkg/sync/atomic/#pkg-note-BUG
nextSpanID uint64
spanIDInc uint64

traceIDAdd [2]uint64
nextSpanID uint64
spanIDInc uint64
traceIDRand *rand.Rand
}

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

0 comments on commit 67abf0e

Please sign in to comment.