From f40927268ff78b7f1730f93be6ae54bf38c09a22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu> Date: Thu, 26 Jan 2017 10:12:34 +0100 Subject: [PATCH] Manager: fixed panic in TimeoutAfter func when timeout channel was closed --- .../flamenco-manager/flamenco/scheduler_test.go | 3 +++ .../flamenco-manager/flamenco/task_updates_test.go | 2 +- .../src/flamenco-manager/flamenco/timer.go | 14 +++++++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/scheduler_test.go b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/scheduler_test.go index a5c9238f..83d819ec 100644 --- a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/scheduler_test.go +++ b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/scheduler_test.go @@ -156,6 +156,7 @@ func (s *SchedulerTestSuite) TestSchedulerVerifyUpstreamCanceled(t *check.C) { } timeout := TimeoutAfter(1 * time.Second) + defer close(timeout) // Mock that the task with highest priority was actually canceled on the Server. httpmock.RegisterResponder( @@ -207,6 +208,7 @@ func (s *SchedulerTestSuite) TestSchedulerVerifyUpstreamPrioChange(t *check.C) { } timeout := TimeoutAfter(1 * time.Second) + defer close(timeout) // Mock that the task with highest priority was actually canceled on the Server. httpmock.RegisterResponder( @@ -264,6 +266,7 @@ func (s *SchedulerTestSuite) TestSchedulerVerifyUpstreamDeleted(t *check.C) { } timeout := TimeoutAfter(1 * time.Second) + defer close(timeout) // Mock that the task with highest priority was actually canceled on the Server. httpmock.RegisterResponder( diff --git a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/task_updates_test.go b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/task_updates_test.go index f912e85f..48b3245d 100644 --- a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/task_updates_test.go +++ b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/task_updates_test.go @@ -53,6 +53,7 @@ func (s *TaskUpdatesTestSuite) TestCancelRunningTasks(t *check.C) { } timeout := TimeoutAfter(1 * time.Second) + defer close(timeout) // Mock that the task with highest priority was actually canceled on the Server. httpmock.RegisterResponder( @@ -81,7 +82,6 @@ func (s *TaskUpdatesTestSuite) TestCancelRunningTasks(t *check.C) { timedout := <-timeout assert.False(t, timedout, "HTTP POST to Flamenco Server not performed") - close(timeout) // Give the tup.Go() coroutine (and subsequent calls) time to run. // the "timeout <- false" call in the responder is triggered before diff --git a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/timer.go b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/timer.go index cbd63858..467ded9e 100644 --- a/packages/flamenco-manager-go/src/flamenco-manager/flamenco/timer.go +++ b/packages/flamenco-manager-go/src/flamenco-manager/flamenco/timer.go @@ -88,11 +88,23 @@ func UtcNow() *time.Time { return &now } +/* Sends a 'true' to the channel after the given timeout. + * Send a 'false' to the channel yourself if you want to notify the receiver that + * a timeout didn't happen. + * + * The channel is buffered with size 2, so both your 'false' and this routine's 'true' + * write won't block. + */ func TimeoutAfter(duration time.Duration) chan bool { - timeout := make(chan bool, 1) + timeout := make(chan bool, 2) go func() { time.Sleep(duration) + defer func() { + // Recover from a panic. This panic can happen when the caller closed the + // channel while we were sleeping. + recover() + }() timeout <- true }() -- GitLab