From ecf5e907835c97910ab01ce3238422ba47fcc095 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Wed, 16 Mar 2022 09:13:45 -0300 Subject: [PATCH] fix(admin-monitor): fix a data race in the admin monitor EE-2761 (#6658) --- api/adminmonitor/admin_monitor.go | 12 ++++++++++++ api/adminmonitor/admin_monitor_test.go | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/api/adminmonitor/admin_monitor.go b/api/adminmonitor/admin_monitor.go index 317b870ca..d2fd673ef 100644 --- a/api/adminmonitor/admin_monitor.go +++ b/api/adminmonitor/admin_monitor.go @@ -3,6 +3,7 @@ package adminmonitor import ( "context" "log" + "sync" "time" portainer "github.com/portainer/portainer/api" @@ -16,6 +17,7 @@ type Monitor struct { datastore dataservices.DataStore shutdownCtx context.Context cancellationFunc context.CancelFunc + mu sync.Mutex } // New creates a monitor that when started will wait for the timeout duration and then shutdown the application unless it has been initialized. @@ -29,6 +31,13 @@ func New(timeout time.Duration, datastore dataservices.DataStore, shutdownCtx co // Starts starts the monitor. Active monitor could be stopped or shuttted down by cancelling the shutdown context. func (m *Monitor) Start() { + m.mu.Lock() + defer m.mu.Unlock() + + if m.cancellationFunc != nil { + return + } + cancellationCtx, cancellationFunc := context.WithCancel(context.Background()) m.cancellationFunc = cancellationFunc @@ -53,6 +62,9 @@ func (m *Monitor) Start() { // Stop stops monitor. Safe to call even if monitor wasn't started. func (m *Monitor) Stop() { + m.mu.Lock() + defer m.mu.Unlock() + if m.cancellationFunc == nil { return } diff --git a/api/adminmonitor/admin_monitor_test.go b/api/adminmonitor/admin_monitor_test.go index 2b32eb921..014e01fb4 100644 --- a/api/adminmonitor/admin_monitor_test.go +++ b/api/adminmonitor/admin_monitor_test.go @@ -21,6 +21,18 @@ func Test_stopCouldBeCalledMultipleTimes(t *testing.T) { monitor.Stop() } +func Test_startOrStopCouldBeCalledMultipleTimesConcurrently(t *testing.T) { + monitor := New(1*time.Minute, nil, context.Background()) + + go monitor.Start() + monitor.Start() + + go monitor.Stop() + monitor.Stop() + + time.Sleep(2 * time.Second) +} + func Test_canStopStartedMonitor(t *testing.T) { monitor := New(1*time.Minute, nil, context.Background()) monitor.Start()