Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Value alignment issue on 32-bit systems (ARM) #43

Open
abferm opened this issue Mar 30, 2017 · 1 comment
Open

Value alignment issue on 32-bit systems (ARM) #43

abferm opened this issue Mar 30, 2017 · 1 comment

Comments

@abferm
Copy link
Contributor

abferm commented Mar 30, 2017

This library suffers from go issue #599 when compiled for ARM using the gc compiler. The gccgo compiler appears to take care of it, but that limits the go version to <=1.6.1 for official gccgo releases. The following padding & reordering of the Breaker struct fixes the issue.

`// Breaker is the base of a circuit breaker. It maintains failure and success counters
// as well as the event subscribers.
type Breaker struct {
// BackOff is the backoff policy that is used when determining if the breaker should
// attempt to retry. A breaker created with NewBreaker will use an exponential backoff
// policy by default.
BackOff backoff.BackOff

// ShouldTrip is a TripFunc that determines whether a Fail() call should trip the breaker.
// A breaker created with NewBreaker will not have a ShouldTrip by default, and thus will
// never automatically trip.
ShouldTrip TripFunc

// Clock is used for controlling time in tests.
Clock clock.Clock

_              [4]byte // pad to fix golang issue #599
consecFailures int64
lastFailure    int64 // stored as nanoseconds since the Unix epoch
halfOpens      int64
counts         *window
nextBackOff    time.Duration
tripped        int32
broken         int32
eventReceivers []chan BreakerEvent
listeners      []chan ListenerEvent
backoffLock    sync.Mutex

}`

abferm added a commit to abferm/circuitbreaker that referenced this issue Mar 30, 2017
Reordered Breaker struct and added padding to ensure that the 64-bit values have 8-byte alignment on 32-bit systems. This is required by the atomic library for values larger than 4 bytes on 32-bit systems and will cause a panic if that alignment is incorrect.
@abferm
Copy link
Contributor Author

abferm commented Mar 30, 2017

The issue also presents itself in 386 binaries. The issue can be demonstrated by compiling the following code for a 64-bit system and a 32-bit system and running both. Switching the import to my fork will make the code work for both 32-bit and 64-bit systems.

package main

import (
	"log"

	circuit "github.com/rubyist/circuitbreaker"
)

func main() {
	cb := circuit.NewThresholdBreaker(10)
	events := cb.Subscribe()
	go func() {
		for {
			e := <-events
			// Monitor breaker events like BreakerTripped, BreakerReset, BreakerFail, BreakerReady
			log.Println(e)
		}
	}()

	cb.Success()
	cb.Fail()
	cb.Reset()
}

rubyist added a commit that referenced this issue Mar 30, 2017
Fix issue #43 [Value alignment issue on 32-bit systems (ARM)]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant