fixes dlclark#63 add the ability for unit tests to wait on the timeout background goroutine

Author: dlclark

README.md

@@ -113,22 +113,44 @@ deadline for the match. The performance impact is as follows.
     is reached. E.g., if you set a timeout of one minute the load will persist
     for approximately a minute even if the match finishes quickly.
 
-Some alternative implementations were considered and ruled out. 
-
-1.  **time.Now()** - This was the initial timeout implementation. It called `time.Now()`
-    and compared the result to the deadline approximately once every 1000 matching steps.
-    Adding a timeout to a simple match increased the cost from ~45ns to ~3000ns).
-2.  **time.AfterFunc** - This approach entails using `time.AfterFunc` to set an `expired`
-    atomic boolean value. However it increases the cost of handling a simple match 
-    with a timeout from ~45ns to ~360ns and was therefore ruled out.
-3.  **counter** - In this approach an atomic variable tracks the number of live matches
-    with timeouts. The background clock stops when the counter hits zero. The benefit
-    of this approach is that the background load will stop more quickly (after the
-    last match has finished as opposed to waiting until the deadline for the last
-    match). However this approach requires more atomic variable updates and has poorer
-    performance when multiple matches are executed concurrently. (The cost of a
-    single match jumps from ~45ns to ~65ns, and the cost of running matches on
-    all 12 available CPUs jumps from ~400ns to ~730ns).
+See [PR #58](https://github.com/dlclark/regexp2/pull/58) for more details and 
+alternatives considered.
+
+## Goroutine leak error
+If you're using a library during unit tests (e.g. https://github.com/uber-go/goleak) that validates all goroutines are exited then you'll likely get an error if you or any of your dependencies use regex's with a MatchTimeout. 
+To remedy the problem you'll need to tell the unit test to wait until the backgroup timeout goroutine is exited.
+
+```go
+func TestSomething(t *testing.T) {
+    defer goleak.VerifyNone(t)
+    defer regexp2.StopTimeoutClock()
+
+    // ... test
+}
+
+//or
+
+func TestMain(m *testing.M) {
+    // setup
+    // ...
+
+    // run 
+    m.Run()
+
+    //tear down
+    regexp2.StopTimeoutClock()
+    goleak.VerifyNone(t)
+}
+```
+
+This will add ~100ms runtime to each test (or TestMain). If that's too much time you can set the clock cycle rate of the timeout goroutine in an init function in a test file. `regexp2.SetTimeoutCheckPeriod` isn't threadsafe so it must be setup before starting any regex's with Timeouts.
+
+```go
+func init() {
+	//speed up testing by making the timeout clock 1ms
+	regexp2.SetTimeoutCheckPeriod(time.Millisecond)
+}
+```
 
 ## ECMAScript compatibility mode
 In this mode the engine provides compatibility with the [regex engine](https://tc39.es/ecma262/multipage/text-processing.html#sec-regexp-regular-expression-objects) described in the ECMAScript specification.

fastclock.go

@@ -76,14 +76,36 @@ func extendClock(end fasttime) {
 	}
 }
 
+// stop the timeout clock in the background
+// should only used for unit tests to abandon the background goroutine
+func stopClock() {
+	fast.mu.Lock()
+	if fast.running {
+		fast.clockEnd.write(fasttime(0))
+	}
+	fast.mu.Unlock()
+
+	// pause until not running
+	// get and release the lock
+	isRunning := true
+	for isRunning {
+		time.Sleep(clockPeriod / 2)
+		fast.mu.Lock()
+		isRunning = fast.running
+		fast.mu.Unlock()
+	}
+}
+
 func durationToTicks(d time.Duration) fasttime {
 	// Downscale nanoseconds to approximately a millisecond so that we can avoid
 	// overflow even if the caller passes in math.MaxInt64.
 	return fasttime(d) >> 20
 }
 
+const DefaultClockPeriod = 100 * time.Millisecond
+
 // clockPeriod is the approximate interval between updates of approximateClock.
-const clockPeriod = 100 * time.Millisecond
+var clockPeriod = DefaultClockPeriod
 
 func runClock() {
 	fast.mu.Lock()

fastclock_test.go

@@ -6,6 +6,11 @@ import (
 	"time"
 )
 
+func init() {
+	//speed up testing by making the timeout clock 1ms instead of 100ms...
+	//bad for benchmark tests though
+	SetTimeoutCheckPeriod(time.Millisecond)
+}
 func TestDeadline(t *testing.T) {
 	for _, delay := range []time.Duration{
 		clockPeriod / 10,
@@ -32,3 +37,22 @@ func TestDeadline(t *testing.T) {
 		})
 	}
 }
+
+func TestStopTimeoutClock(t *testing.T) {
+	// run a quick regex with a long timeout
+	// make sure the stop clock returns quickly
+	r := MustCompile(".", 0)
+	r.MatchTimeout = time.Second * 10
+
+	r.MatchString("a")
+	start := time.Now()
+	StopTimeoutClock()
+	stop := time.Now()
+
+	if want, got := clockPeriod*2, stop.Sub(start); want < got {
+		t.Errorf("Expected duration less than %v, got %v", want, got)
+	}
+	if want, got := false, fast.running; want != got {
+		t.Errorf("Expected isRunning to be %v, got %v", want, got)
+	}
+}

regexp.go

@@ -96,6 +96,19 @@ func Unescape(input string) (string, error) {
 	return syntax.Unescape(input)
 }
 
+// SetTimeoutPeriod is a debug function that sets the frequency of the timeout goroutine's sleep cycle.
+// Defaults to 100ms. The only benefit of setting this lower is that the 1 background goroutine that manages
+// timeouts may exit slightly sooner after all the timeouts have expired. See Github issue #63
+func SetTimeoutCheckPeriod(d time.Duration) {
+	clockPeriod = d
+}
+
+// StopTimeoutClock should only be used in unit tests to prevent the timeout clock goroutine
+// from appearing like a leaking goroutine
+func StopTimeoutClock() {
+	stopClock()
+}
+
 // String returns the source text used to compile the regular expression.
 func (re *Regexp) String() string {
 	return re.pattern

regexp_performance_test.go

@@ -310,6 +310,9 @@ func BenchmarkLeading(b *testing.B) {
 }
 
 func BenchmarkShortSearch(b *testing.B) {
+	// set to default check period for benchmarking purposes
+	// unit tests tend to send this low
+	SetTimeoutCheckPeriod(DefaultClockPeriod)
 	type benchmark struct {
 		name     string
 		parallel bool // Run in parallel?

regexp_test.go

@@ -19,7 +19,7 @@ func TestBacktrack_CatastrophicTimeout(t *testing.T) {
 	const subject = "Do you think you found the problem string!"
 
 	const earlyAllowance = 10 * time.Millisecond
-	const lateAllowance = clockPeriod + 500*time.Millisecond // Large allowance in case machine is slow
+	var lateAllowance = clockPeriod + 500*time.Millisecond // Large allowance in case machine is slow
 
 	for _, timeout := range []time.Duration{
 		-1 * time.Millisecond,