From 40ae6a40a970ef4cdbffa7b24b280e316db8accc Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Tue, 19 Nov 2019 13:33:40 -0800 Subject: [PATCH] Modify Bool to only use 1 or 0, rater than last bit (#62) 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. --- CHANGELOG.md | 7 ++++--- atomic.go | 9 +++++++-- atomic_test.go | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d84d5a..6db846f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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 diff --git a/atomic.go b/atomic.go index 1db6849..ad5fa09 100644 --- a/atomic.go +++ b/atomic.go @@ -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 { diff --git a/atomic_test.go b/atomic_test.go index 6666f8a..55e220f 100644 --- a/atomic_test.go +++ b/atomic_test.go @@ -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")