From 84d56122779572ab483220e8b14a64f267185547 Mon Sep 17 00:00:00 2001
From: Marc Coury <gloriphobia@gmail.com>
Date: Wed, 29 Jan 2020 12:46:16 +0000
Subject: [PATCH] Fix race condition from CancelAll (#5)

---
 CHANGELOG.md     | 35 +++++++++++++++++++----------------
 README.md        | 26 ++++++++++++++++++++++++++
 rununtil.go      | 12 +++---------
 rununtil_test.go | 10 ++++++++++
 4 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b2b58aa..4d863e4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,34 +1,37 @@
-# Changelog
-All notable changes to this project will be documented in this file.
+# Change Log
 
-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).
+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]
-[Unreleased]: https://github.com/mec07/rununtil/compare/v0.2.0...HEAD
+## [0.2.2] - 2020-01-29
+
+### Fixed
+
+- Fix race condition in CancelAll
 
 ## [0.2.0] - 2019-06-12
-[0.2.0]: https://github.com/mec07/rununtil/compare/v0.1.0...v0.2.0
+
+### Fixed
+
+- There was a problem with sending a kill signal to a nonblocking main function. This has been fixed by providing the CancelAll method.
+
 ### Changed
+
 - Deprecate: functions KillSignal, Signals, and Killed
 - Replace them with: AwaitKillSignal, AwaitKillSignals and CancelAll
 
-### Fixed
-- There was a problem with sending a kill signal to a nonblocking main function.
-  This has been fixed by providing the CancelAll method.
-
 ## [0.1.0] - 2019-06-12
-[0.1.0]: https://github.com/mec07/rununtil/compare/v0.0.1...v0.1.0
-### Changed
-- There are now RunnerFunc and ShutdownFunc function types to clarify the usage of this library (backwards incompatible change)
 
 ### Added
+
 - Implemented the rununtil.Killed method, which allows you to test a function that uses rununtil.KillSignal
 
+### Changed
+
+- There are now RunnerFunc and ShutdownFunc function types to clarify the usage of this library (backwards incompatible change)
 
 ## [0.0.1] - 2019-06-05
-[0.0.1]: https://github.com/mec07/rununtil/releases/tag/v0.0.1
+
 ### Added
-- initial commit of rununtil library
 
+- initial commit of rununtil library
 
diff --git a/README.md b/README.md
index 7995838..14b1bc3 100644
--- a/README.md
+++ b/README.md
@@ -2,3 +2,29 @@
 Go library to run a function until a kill signal is recieved.
 
 See the docs: https://godoc.org/github.com/mec07/rununtil
+
+## Changelog
+
+The [CHANGELOG.md](./CHANGELOG.md) file tracks all of the changes and each release.
+We are managing it using the helpful [changelog-tool](https://github.com/ponylang/changelog-tool).
+On Mac you can install it with brew: `brew install kaluza-tech/devint/changelog-tool`.
+On linux (or any other platform) you have to install the pony language.
+Then just follow the instructions on the github page for installing the changelog-tool:
+https://github.com/ponylang/changelog-tool#installation
+
+The use of the tool is straightforward.
+To create a new changelog (don't run this in this repo because then you'll replace the current changelog with a new one!):
+```
+changelog-tool new
+```
+To start recording a new entry:
+```
+changelog-tool unreleased -e
+```
+The `-e` means update the changelog file in place.
+Then manually edit the changelog to add your changes in the unreleased section.
+When you're ready you can then "release" it by executing:
+```
+changelog-tool release 0.0.1 -e
+```
+Replace `0.0.1` with the new version.
diff --git a/rununtil.go b/rununtil.go
index 0b211f3..e9a6ad3 100644
--- a/rununtil.go
+++ b/rununtil.go
@@ -95,22 +95,17 @@ type canceller struct {
 
 func (canc *canceller) addChannel(key string, c chan struct{}) {
 	canc.mux.Lock()
+	defer canc.mux.Unlock()
 	canc.signals[key] = c
-	canc.mux.Unlock()
-}
-
-func (canc *canceller) removeChannel(key string) {
-	canc.mux.Lock()
-	delete(canc.signals, key)
-	canc.mux.Unlock()
 }
 
 func (canc *canceller) cancelAll() {
 	canc.mux.Lock()
+	defer canc.mux.Unlock()
 	for key := range canc.signals {
 		close(canc.signals[key])
+		delete(canc.signals, key)
 	}
-	canc.mux.Unlock()
 }
 
 var globalCanceller canceller
@@ -146,7 +141,6 @@ func AwaitKillSignals(signals []os.Signal, runnerFuncs ...RunnerFunc) {
 	finish := make(chan struct{})
 	uuid := uuid.New()
 	globalCanceller.addChannel(uuid.String(), finish)
-	defer globalCanceller.removeChannel(uuid.String())
 
 	for _, runner := range runnerFuncs {
 		shutdown := runner()
diff --git a/rununtil_test.go b/rununtil_test.go
index 602a5f5..7fc822c 100644
--- a/rununtil_test.go
+++ b/rununtil_test.go
@@ -147,9 +147,19 @@ func TestRununtilCancelAll_MultipleTimes(t *testing.T) {
 }
 
 func TestRununtilCancelAll_Threadsafe(t *testing.T) {
+	var hasBeenKilledVec [100]bool
 	for idx := 0; idx < 100; idx++ {
+		cancel := rununtil.Killed(helperMakeMain(&hasBeenKilledVec[idx]))
+		cancel()
 		rununtil.CancelAll()
 	}
+	// yield control back to scheduler so that killing can actually happen
+	time.Sleep(time.Millisecond)
+	for idx, hasBeenKilled := range hasBeenKilledVec {
+		if !hasBeenKilled {
+			t.Fatalf("expected main to have been killed: %d", idx)
+		}
+	}
 }
 
 // Annoyingly this test has to be run by itself to actually fail...