Skip to content

Commit

Permalink
profiling: Fix a data race in the circular buffer. (#3254)
Browse files Browse the repository at this point in the history
  • Loading branch information
easwars authored Dec 13, 2019
1 parent e3baa76 commit f7b39d8
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
8 changes: 5 additions & 3 deletions internal/profiling/buffer/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*
*/

// Package buffer provides a high-performant lock free implementation of a
// circular buffer used by the profiling code.
package buffer

import (
Expand All @@ -42,7 +44,7 @@ type queue struct {
// proceeding forwarding with the actual write to arr. This counter is also
// used by the Drain operation's drainWait subroutine to wait for all pushes
// to complete.
acquired uint32
acquired uint32 // Accessed atomically.
// After the completion of a Push operation, the written counter is
// incremented. Also used by drainWait to wait for all pushes to complete.
written uint32
Expand Down Expand Up @@ -255,8 +257,8 @@ func (cb *CircularBuffer) Drain() []interface{} {

result := make([]interface{}, 0)
for i := 0; i < len(qs); i++ {
if qs[i].acquired < qs[i].size {
result = dereferenceAppend(result, qs[i].arr, 0, qs[i].acquired)
if acquired := atomic.LoadUint32(&qs[i].acquired); acquired < qs[i].size {
result = dereferenceAppend(result, qs[i].arr, 0, acquired)
} else {
result = dereferenceAppend(result, qs[i].arr, 0, qs[i].size)
}
Expand Down
11 changes: 8 additions & 3 deletions internal/profiling/buffer/buffer_appengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,26 @@
*
*/

// Appengine does not support stats because of lack of support for unsafe
package buffer

// CircularBuffer is a no-op implementation for appengine builds.
//
// Appengine does not support stats because of lack of the support for unsafe
// pointers, which are necessary to efficiently store and retrieve things into
// and from a circular buffer. As a result, Push does not do anything and Drain
// returns an empty slice.
package buffer

type CircularBuffer struct{}

// NewCircularBuffer returns a no-op for appengine builds.
func NewCircularBuffer(size uint32) (*CircularBuffer, error) {
return nil, nil
}

// Push returns a no-op for appengine builds.
func (cb *CircularBuffer) Push(x interface{}) {
}

// Drain returns a no-op for appengine builds.
func (cb *CircularBuffer) Drain() []interface{} {
return nil
}
3 changes: 1 addition & 2 deletions internal/profiling/profiling.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*
*/

// The profiling package contains two logical components: buffer.go and
// Package profiling contains two logical components: buffer.go and
// profiling.go. The former implements a circular buffer (a.k.a. ring buffer)
// in a lock-free manner using atomics. This ring buffer is used by
// profiling.go to store various statistics. For example, StreamStats is a
Expand All @@ -30,7 +30,6 @@
// more types of measurements (such as the number of memory allocations) could
// be measured, which might require a different type of object being pushed
// into the circular buffer.

package profiling

import (
Expand Down

0 comments on commit f7b39d8

Please sign in to comment.