From 4537cf7b6c719d74bd14a7b9aa148d0902ed6788 Mon Sep 17 00:00:00 2001 From: Joe Turki Date: Wed, 26 Nov 2025 20:11:09 +0200 Subject: [PATCH] Fix a deadlock in TaskLoop --- internal/taskloop/taskloop.go | 2 + internal/taskloop/taskloop_test.go | 60 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 internal/taskloop/taskloop_test.go diff --git a/internal/taskloop/taskloop.go b/internal/taskloop/taskloop.go index 63e780f..4c8e43e 100644 --- a/internal/taskloop/taskloop.go +++ b/internal/taskloop/taskloop.go @@ -85,6 +85,8 @@ func (l *Loop) Run(ctx context.Context, t func(context.Context)) error { select { case <-ctx.Done(): return ctx.Err() + case <-l.done: + return ErrClosed case l.tasks <- task{t, done}: <-done diff --git a/internal/taskloop/taskloop_test.go b/internal/taskloop/taskloop_test.go new file mode 100644 index 0000000..00c3c64 --- /dev/null +++ b/internal/taskloop/taskloop_test.go @@ -0,0 +1,60 @@ +// SPDX-FileCopyrightText: 2023 The Pion community +// SPDX-License-Identifier: MIT + +package taskloop + +import ( + "context" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestRunReturnsErrClosedWhenLoopClosing(t *testing.T) { + loop := New(func() {}) + + blockStarted := make(chan struct{}) + releaseBlock := make(chan struct{}) + go func() { + _ = loop.Run(context.Background(), func(context.Context) { + close(blockStarted) + <-releaseBlock + }) + }() + <-blockStarted + + var secondRan atomic.Bool + errCh := make(chan error, 1) + go func() { + errCh <- loop.Run(context.Background(), func(context.Context) { + secondRan.Store(true) + }) + }() + + time.Sleep(10 * time.Millisecond) + + closeDone := make(chan struct{}) + go func() { + loop.Close() + close(closeDone) + }() + + select { + case err := <-errCh: + assert.ErrorIs(t, err, ErrClosed) + case <-time.After(time.Second): + assert.Fail(t, "Run did not return after loop close") + } + + close(releaseBlock) + + select { + case <-closeDone: + case <-time.After(time.Second): + assert.Fail(t, "Close did not return") + } + + assert.False(t, secondRan.Load(), "second task should not excute after loop is closed") +}