Skip to content

Commit

Permalink
Modify Bool to only use 1 or 0, rater than last bit (#62)
Browse files Browse the repository at this point in the history
Most methods of Bool currently rely on the last bit, but `CAS`
generates the int value to match `old` based on the user's input
and so it assumes the value is either `1` or `0`.

The only reason we relid on the last bit is that `Toggle` was
implemented using an atomic add. We can instead implement
`Toggle` using a loop around `CAS`.

Fixes #61.
  • Loading branch information
prashantv authored Nov 19, 2019
1 parent b99530f commit 40ae6a4
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 6 deletions.
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- No changes yet.
## [1.5.1] - 2019-11-19
- Fix bug where `Bool.CAS` and `Bool.Toggle` do work correctly together
causing `CAS` to fail even though the old value matches.

## [1.5.0] - 2019-10-29
### Changed
Expand Down Expand Up @@ -47,7 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Initial release.

[Unreleased]: https://github.com/uber-go/atomic/compare/v1.5.0...HEAD
[1.5.1]: https://github.com/uber-go/atomic/compare/v1.5.0...v1.5.1
[1.5.0]: https://github.com/uber-go/atomic/compare/v1.4.0...v1.5.0
[1.4.0]: https://github.com/uber-go/atomic/compare/v1.3.2...v1.4.0
[1.3.2]: https://github.com/uber-go/atomic/compare/v1.3.1...v1.3.2
Expand Down
9 changes: 7 additions & 2 deletions atomic.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,16 @@ func (b *Bool) Swap(new bool) bool {

// Toggle atomically negates the Boolean and returns the previous value.
func (b *Bool) Toggle() bool {
return truthy(atomic.AddUint32(&b.v, 1) - 1)
for {
old := b.Load()
if b.CAS(old, !old) {
return old
}
}
}

func truthy(n uint32) bool {
return n&1 == 1
return n == 1
}

func boolToInt(b bool) uint32 {
Expand Down
4 changes: 3 additions & 1 deletion atomic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ func TestUint64(t *testing.T) {

func TestBool(t *testing.T) {
atom := NewBool(false)
require.False(t, atom.Toggle(), "Expected swap to return previous value.")
require.False(t, atom.Toggle(), "Expected Toggle to return previous value.")
require.True(t, atom.Toggle(), "Expected Toggle to return previous value.")
require.False(t, atom.Toggle(), "Expected Toggle to return previous value.")
require.True(t, atom.Load(), "Unexpected state after swap.")

require.True(t, atom.CAS(true, true), "CAS should swap when old matches")
Expand Down

0 comments on commit 40ae6a4

Please sign in to comment.