mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-21 04:57:48 +00:00
R88: make StopAllTasks cover ResourceManager singleton tasks
TaskRegistry.CreateSingletonTask delegates to ResourceManager.RegisterBackgroundTask and stores the task in the ResourceManager's registry, not TaskRegistry.tasks. TaskRegistry.StopAllTasks previously iterated its own (always-empty) registry, silently leaking every process-global singleton task's ticker goroutine past the last Close (memory-monitor, replay-cache-cleanup) — tasks kept running real periodic work with no instance to serve. Add ResourceManager.StopAllTasks (stops all rm-owned tasks) and have TaskRegistry.StopAllTasks stop both its own RegisterTask-direct tasks and the ResourceManager-owned singletons. Regression TestR88StopAllTasksStopsResourceManagerOwnedTasks (review_r88_test.go): fails on old code (task still running after StopAllTasks), passes on new.
This commit is contained in:
+18
-7
@@ -518,9 +518,18 @@ func (tr *TaskRegistry) GetTask(name string) (*BackgroundTask, bool) {
|
|||||||
return task, exists
|
return task, exists
|
||||||
}
|
}
|
||||||
|
|
||||||
// StopAllTasks stops all registered background tasks
|
// StopAllTasks stops all registered background tasks.
|
||||||
|
//
|
||||||
|
// Tasks can live in two places: those registered directly via
|
||||||
|
// TaskRegistry.RegisterTask (stored in tr.tasks) and process-global
|
||||||
|
// singletons created via TaskRegistry.CreateSingletonTask, which delegate to
|
||||||
|
// GetResourceManager().RegisterBackgroundTask and store the task in the
|
||||||
|
// ResourceManager's registry (see CreateSingletonTask below) — NOT in
|
||||||
|
// tr.tasks. Stop both so neither a direct task nor a singleton task leaks its
|
||||||
|
// ticker goroutine past the last Close.
|
||||||
func (tr *TaskRegistry) StopAllTasks() {
|
func (tr *TaskRegistry) StopAllTasks() {
|
||||||
// First, copy the tasks map to avoid deadlock with GetTaskCount()
|
// First, stop tasks registered directly in this registry.
|
||||||
|
// Copy the tasks map to avoid deadlock with GetTaskCount().
|
||||||
tr.mu.Lock()
|
tr.mu.Lock()
|
||||||
tasksCopy := make(map[string]*BackgroundTask, len(tr.tasks))
|
tasksCopy := make(map[string]*BackgroundTask, len(tr.tasks))
|
||||||
for name, task := range tr.tasks {
|
for name, task := range tr.tasks {
|
||||||
@@ -530,13 +539,15 @@ func (tr *TaskRegistry) StopAllTasks() {
|
|||||||
tr.tasks = make(map[string]*BackgroundTask)
|
tr.tasks = make(map[string]*BackgroundTask)
|
||||||
tr.mu.Unlock()
|
tr.mu.Unlock()
|
||||||
|
|
||||||
// Now stop all tasks without holding the lock
|
for _, task := range tasksCopy {
|
||||||
for name, task := range tasksCopy {
|
|
||||||
task.Stop()
|
task.Stop()
|
||||||
if tr.logger != nil {
|
|
||||||
tr.logger.Debug("Stopped background task during shutdown: %s", name)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Then stop process-global singleton tasks, which live in the
|
||||||
|
// ResourceManager's registry, not in tr.tasks (otherwise the singleton
|
||||||
|
// task teardown would be a silent no-op and leak every singleton task's
|
||||||
|
// ticker goroutine past the last Close).
|
||||||
|
GetResourceManager().StopAllTasks()
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetTaskCount returns the number of active tasks
|
// GetTaskCount returns the number of active tasks
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
package traefikoidc
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"sync"
|
||||||
|
"sync/atomic"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// R88 regression: process-global singleton tasks are created via
|
||||||
|
// TaskRegistry.CreateSingletonTask but stored in the ResourceManager's
|
||||||
|
// registry, not in TaskRegistry.tasks. TaskRegistry.StopAllTasks previously
|
||||||
|
// iterated its own (always-empty) registry, silently leaking every singleton
|
||||||
|
// task's ticker goroutine past the last Close (memory-monitor,
|
||||||
|
// replay-cache-cleanup). It must now delegate to the ResourceManager that
|
||||||
|
// actually owns the tasks so a last-instance shutdown stops them.
|
||||||
|
func TestR88StopAllTasksStopsResourceManagerOwnedTasks(t *testing.T) {
|
||||||
|
rm := GetResourceManager()
|
||||||
|
reg := GetGlobalTaskRegistry()
|
||||||
|
|
||||||
|
// Snapshot the currently-running singleton tasks so we can re-create them
|
||||||
|
// after the global teardown below and leave the shared suite un-polluted.
|
||||||
|
type restore struct {
|
||||||
|
name string
|
||||||
|
interval time.Duration
|
||||||
|
fn func()
|
||||||
|
logger *Logger
|
||||||
|
wg *sync.WaitGroup
|
||||||
|
}
|
||||||
|
var toRestore []restore
|
||||||
|
rm.tasksMu.RLock()
|
||||||
|
for name, task := range rm.tasks {
|
||||||
|
if atomic.LoadInt32(&task.started) == 1 && atomic.LoadInt32(&task.stopped) == 0 {
|
||||||
|
toRestore = append(toRestore, restore{name, task.interval, task.taskFunc, task.logger, task.externalWG})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
rm.tasksMu.RUnlock()
|
||||||
|
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
name := fmt.Sprintf("shutdown-probe-%d", time.Now().UnixNano())
|
||||||
|
if _, err := reg.CreateSingletonTask(name, time.Hour, func() {}, newNoOpLogger(), &wg); err != nil {
|
||||||
|
t.Fatalf("CreateSingletonTask: %v", err)
|
||||||
|
}
|
||||||
|
if !rm.IsTaskRunning(name) {
|
||||||
|
t.Fatalf("expected singleton task %q to be running after CreateSingletonTask", name)
|
||||||
|
}
|
||||||
|
|
||||||
|
// This is the exact shutdown path Close() relies on
|
||||||
|
// (utilities.go: lastInstance -> GetGlobalTaskRegistry().StopAllTasks()).
|
||||||
|
reg.StopAllTasks()
|
||||||
|
|
||||||
|
if rm.IsTaskRunning(name) {
|
||||||
|
t.Fatalf("singleton task %q still running after StopAllTasks: global task teardown is a no-op (task lived in ResourceManager, registry iterated its own empty map)", name)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Restore the pre-existing running singletons so this global teardown does
|
||||||
|
// not destabilize sibling tests that still hold live instances.
|
||||||
|
for _, r := range toRestore {
|
||||||
|
if _, err := reg.CreateSingletonTask(r.name, r.interval, r.fn, r.logger, r.wg); err != nil {
|
||||||
|
t.Errorf("restore task %q: %v", r.name, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -178,6 +178,24 @@ func (rm *ResourceManager) StopBackgroundTask(name string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// StopAllTasks stops every background task registered with this ResourceManager.
|
||||||
|
// BackgroundTask.Stop is stopOnce-guarded, so repeated calls are safe. This is
|
||||||
|
// the terminal path for process-global singleton tasks; it must be called only
|
||||||
|
// when the last plugin instance is shutting down, otherwise it would kill
|
||||||
|
// cleanup for surviving instances sharing the same singletons.
|
||||||
|
func (rm *ResourceManager) StopAllTasks() {
|
||||||
|
rm.tasksMu.RLock()
|
||||||
|
tasks := make([]*BackgroundTask, 0, len(rm.tasks))
|
||||||
|
for _, task := range rm.tasks {
|
||||||
|
tasks = append(tasks, task)
|
||||||
|
}
|
||||||
|
rm.tasksMu.RUnlock()
|
||||||
|
|
||||||
|
for _, task := range tasks {
|
||||||
|
task.Stop()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// IsTaskRunning checks if a background task is running
|
// IsTaskRunning checks if a background task is running
|
||||||
func (rm *ResourceManager) IsTaskRunning(name string) bool {
|
func (rm *ResourceManager) IsTaskRunning(name string) bool {
|
||||||
rm.tasksMu.RLock()
|
rm.tasksMu.RLock()
|
||||||
|
|||||||
Reference in New Issue
Block a user