[KLC-2402] appStatusPolling: make Poll stoppable; wire shutdown (#57)

* [KLC-2402] appStatusPolling: make Poll stoppable; wire shutdown

Replaces the unbounded `for { Sleep; ... }` goroutine in AppStatusPolling
with a select over a `done` channel and a ticker, exposing an idempotent
`Close() error` (io.Closer). Killed a pre-existing nil-pointer panic in
TestStartStatusPolling/Valid_configuration, where the leaked goroutine
outlived the test and fired against a stub with a nil method handler.

API:
 - metrics.StartStatusPolling, StartNodeMetricsPolling, and
   StartMachineStatisticsPolling now return (io.Closer, error).

Shutdown:
 - cmd/node/startup.go captures the three closers and passes them to
   closeAllComponents, which now Closes them first — before tearing down
   the components their handlers read from (NetMessenger, Store, etc.).
 - Close signals stop but does not block on the goroutine, per the
   ticket's requirement.

Tests:
 - matics_test.go and appStatusPolling_test.go register t.Cleanup to
   Close pollers, eliminating the test goroutine leak.
 - Added TestAppStatusPolling_Close_IsIdempotent and
   TestAppStatusPolling_Close_StopsGoroutine.

Out of scope (separate ticket): the leaky `for {}` goroutines inside
registerCPUStatistics / registerNetStatistics / registerDiskStatistics.

* fix(KLC-2402): address review feedback

- StartStatusPolling: check.IfNil(ash) for typed-nil safety
- StartMachineStatisticsPolling: add check.IfNil(notifier) guard
- Wrap NewAppStatusPolling error with %w in both Start* wrappers
- Test: fix duration multiplication typo (~63yr -> 2s)
- Test: use require.Eventually for first-tick wait; settling-window check for post-Close stability

Author: fbsobreira

cmd/node/metrics/machineStatistics.go

@@ -2,6 +2,8 @@ package metrics
 
 import (
 	"errors"
+	"fmt"
+	"io"
 	"path/filepath"
 	"time"
 
@@ -13,40 +15,45 @@ import (
 	"github.com/klever-io/klever-go/tools/check"
 )
 
-// StartMachineStatisticsPolling will start read information about current running machine
-func StartMachineStatisticsPolling(ash core.AppStatusHandler, notifier eventNotifier.EpochStartEventNotifier, pollingInterval time.Duration, workingDir string) error {
+// StartMachineStatisticsPolling will start read information about current running machine.
+// The returned io.Closer stops the AppStatusPolling goroutine; the inner goroutines in
+// registerCPUStatistics / registerNetStatistics / registerDiskStatistics still leak
+func StartMachineStatisticsPolling(ash core.AppStatusHandler, notifier eventNotifier.EpochStartEventNotifier, pollingInterval time.Duration, workingDir string) (io.Closer, error) {
 	if check.IfNil(ash) {
-		return errors.New("nil AppStatusHandler")
+		return nil, errors.New("nil AppStatusHandler")
+	}
+	if check.IfNil(notifier) {
+		return nil, errors.New("nil EpochStartEventNotifier")
 	}
 
 	appStatusPollingHandler, err := appStatusPolling.NewAppStatusPolling(ash, pollingInterval)
 	if err != nil {
-		return errors.New("cannot init AppStatusPolling")
+		return nil, fmt.Errorf("cannot init AppStatusPolling: %w", err)
 	}
 
 	err = registerCPUStatistics(appStatusPollingHandler)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	err = registerMemStatistics(appStatusPollingHandler)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	err = registerNetStatistics(appStatusPollingHandler, notifier)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	err = registerDiskStatistics(appStatusPollingHandler, workingDir)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	appStatusPollingHandler.Poll()
 
-	return nil
+	return appStatusPollingHandler, nil
 }
 
 func registerMemStatistics(appStatusPollingHandler *appStatusPolling.AppStatusPolling) error {

cmd/node/metrics/matics_test.go

@@ -262,7 +262,7 @@ func TestStartStatusPolling(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Call the function to test
-			err := StartStatusPolling(
+			closer, err := StartStatusPolling(
 				tt.appStatusHandler,
 				time.Second,
 				tt.networkComponents,
@@ -272,8 +272,11 @@ func TestStartStatusPolling(t *testing.T) {
 			// Assert the result
 			if tt.expectedError {
 				assert.Error(t, err)
+				assert.Nil(t, closer)
 			} else {
 				assert.Nil(t, err)
+				require.NotNil(t, closer)
+				t.Cleanup(func() { _ = closer.Close() })
 			}
 		})
 	}
@@ -451,6 +454,7 @@ func TestRegisterPollConnectedPeers(t *testing.T) {
 	assert.NotPanics(t, func() {
 		pollingHandler.Poll()
 	})
+	t.Cleanup(func() { _ = pollingHandler.Close() })
 }
 
 // TestRegisterPollProbableHighestNonce tests the function that registers a polling function for the probable highest nonce
@@ -485,6 +489,7 @@ func TestRegisterPollProbableHighestNonce(t *testing.T) {
 	assert.NotPanics(t, func() {
 		pollingHandler.Poll()
 	})
+	t.Cleanup(func() { _ = pollingHandler.Close() })
 
 	// Optionally, verify that the metric gets set when Poll() is called
 	// This requires a metricsMap to track what values are set
@@ -498,6 +503,7 @@ func TestRegisterPollProbableHighestNonce(t *testing.T) {
 	pollingWithMetricsHandler, _ := appStatusPolling.NewAppStatusPolling(metricsHandler, time.Second)
 	_ = registerPollProbableHighestNonce(pollingWithMetricsHandler, processComponents)
 	pollingWithMetricsHandler.Poll()
+	t.Cleanup(func() { _ = pollingWithMetricsHandler.Close() })
 
 	// Verify that the metric was set correctly
 	// Note: This might not be reliable depending on the implementation of AppStatusPolling
@@ -553,16 +559,18 @@ func TestStartNodeMetricsPolling_NilHandler(t *testing.T) {
 	t.Parallel()
 
 	redundancyHandler := &consensusMock.NodeRedundancyHandlerStub{}
-	err := StartNodeMetricsPolling(nil, time.Second, redundancyHandler)
+	closer, err := StartNodeMetricsPolling(nil, time.Second, redundancyHandler)
 	assert.Error(t, err)
+	assert.Nil(t, closer)
 }
 
 func TestStartNodeMetricsPolling_NilRedundancyHandler(t *testing.T) {
 	t.Parallel()
 
 	handler := &mock.AppStatusHandlerStub{}
-	err := StartNodeMetricsPolling(handler, time.Second, nil)
+	closer, err := StartNodeMetricsPolling(handler, time.Second, nil)
 	assert.Error(t, err)
+	assert.Nil(t, closer)
 }
 
 func TestStartNodeMetricsPolling_Valid(t *testing.T) {
@@ -583,8 +591,10 @@ func TestStartNodeMetricsPolling_Valid(t *testing.T) {
 		GetSlotsOfInactivityCalled: func() uint64 { return 3 },
 	}
 
-	err := StartNodeMetricsPolling(handler, time.Second, redundancyHandler)
+	closer, err := StartNodeMetricsPolling(handler, time.Second, redundancyHandler)
 	assert.NoError(t, err)
+	require.NotNil(t, closer)
+	t.Cleanup(func() { _ = closer.Close() })
 
 	// Start timestamp should be set immediately (non-zero)
 	mu.Lock()

cmd/node/metrics/metrics.go

@@ -3,6 +3,7 @@ package metrics
 import (
 	"errors"
 	"fmt"
+	"io"
 	"sort"
 	"strings"
 	"time"
@@ -122,41 +123,42 @@ func SaveStringMetric(ash core.AppStatusHandler, key, value string) {
 	ash.SetStringValue(key, value)
 }
 
-// StartStatusPolling will start save information in status handler about network
+// StartStatusPolling will start save information in status handler about network.
+// The returned io.Closer stops the polling goroutine.
 func StartStatusPolling(
 	ash core.AppStatusHandler,
 	pollingInterval time.Duration,
 	networkComponents *factory.NetworkComponents,
 	processComponents *factory.Process,
-) error {
-	if ash == nil {
-		return errors.New("nil AppStatusHandler")
+) (io.Closer, error) {
+	if check.IfNil(ash) {
+		return nil, errors.New("nil AppStatusHandler")
 	}
 	if networkComponents == nil {
-		return errors.New("nil networkComponents")
+		return nil, errors.New("nil networkComponents")
 	}
 	if processComponents == nil {
-		return errors.New("nil processComponents")
+		return nil, errors.New("nil processComponents")
 	}
 
 	appStatusPollingHandler, err := appStatusPolling.NewAppStatusPolling(ash, pollingInterval)
 	if err != nil {
-		return errors.New("cannot init AppStatusPolling")
+		return nil, fmt.Errorf("cannot init AppStatusPolling: %w", err)
 	}
 
 	err = registerPollConnectedPeers(appStatusPollingHandler, networkComponents)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	err = registerPollProbableHighestNonce(appStatusPollingHandler, processComponents)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	appStatusPollingHandler.Poll()
 
-	return nil
+	return appStatusPollingHandler, nil
 }
 
 func registerPollConnectedPeers(
@@ -234,38 +236,39 @@ func setCurrentP2pNodeAddresses(
 
 // StartNodeMetricsPolling starts polling for uptime and redundancy metrics on a single shared
 // polling goroutine, avoiding the overhead of separate AppStatusPolling instances for each.
+// The returned io.Closer stops the polling goroutine.
 func StartNodeMetricsPolling(
 	ash core.AppStatusHandler,
 	pollingInterval time.Duration,
 	redundancyHandler consensus.NodeRedundancyHandler,
-) error {
+) (io.Closer, error) {
 	if check.IfNil(ash) {
-		return errors.New("nil AppStatusHandler")
+		return nil, errors.New("nil AppStatusHandler")
 	}
 	if check.IfNil(redundancyHandler) {
-		return errors.New("nil NodeRedundancyHandler")
+		return nil, errors.New("nil NodeRedundancyHandler")
 	}
 
 	startTime := time.Now()
 	ash.SetUInt64Value(core.MetricNodeStartTimestamp, uint64(startTime.Unix())) // #nosec G115
 
 	appStatusPollingHandler, err := appStatusPolling.NewAppStatusPolling(ash, pollingInterval)
 	if err != nil {
-		return fmt.Errorf("cannot init AppStatusPolling for node metrics: %w", err)
+		return nil, fmt.Errorf("cannot init AppStatusPolling for node metrics: %w", err)
 	}
 
 	pollingFunc := buildNodeMetricsPollingFunc(startTime, redundancyHandler)
 	err = appStatusPollingHandler.RegisterPollingFunc(pollingFunc)
 	if err != nil {
-		return fmt.Errorf("cannot register node metrics polling function: %w", err)
+		return nil, fmt.Errorf("cannot register node metrics polling function: %w", err)
 	}
 
 	// Prime before the recurring poll so /node/status returns the real level on
 	// request 1 (AppStatusPolling.Poll sleeps before its first tick).
 	pollingFunc(ash)
 
 	appStatusPollingHandler.Poll()
-	return nil
+	return appStatusPollingHandler, nil
 }
 
 func buildNodeMetricsPollingFunc(

cmd/node/startup.go

@@ -967,7 +967,7 @@ func startNode(ctx *cli.Context, log logger.Logger, version string) error {
 
 	log.Trace("starting status pooling components")
 	statusPollingInterval := time.Duration(cfg.Preferences.StatusPollingIntervalSec) * time.Second
-	err = metrics.StartStatusPolling(
+	statusPollingCloser, err := metrics.StartStatusPolling(
 		currentNode.GetAppStatusHandler(),
 		statusPollingInterval,
 		networkComponents,
@@ -978,16 +978,19 @@ func startNode(ctx *cli.Context, log logger.Logger, version string) error {
 	}
 
 	updateMachineStatisticsDuration := time.Second
-	err = metrics.StartMachineStatisticsPolling(coreComponents.StatusHandler, epochStartNotifier, updateMachineStatisticsDuration, workingDir)
+	machineStatsCloser, err := metrics.StartMachineStatisticsPolling(coreComponents.StatusHandler, epochStartNotifier, updateMachineStatisticsDuration, workingDir)
 	if err != nil {
 		return err
 	}
 
-	err = metrics.StartNodeMetricsPolling(coreComponents.StatusHandler, statusPollingInterval, nodeRedundancy)
+	nodeMetricsCloser, err := metrics.StartNodeMetricsPolling(coreComponents.StatusHandler, statusPollingInterval, nodeRedundancy)
 	if err != nil {
 		return err
 	}
 
+	// Pollers are independent; order doesn't matter.
+	pollingClosers := []io.Closer{statusPollingCloser, machineStatsCloser, nodeMetricsCloser}
+
 	log.Trace("creating klever node facade")
 	restAPIServerDebugMode := ctx.GlobalBool(restAPIDebug.Name)
 
@@ -1041,7 +1044,7 @@ func startNode(ctx *cli.Context, log logger.Logger, version string) error {
 
 	chanCloseComponents := make(chan struct{})
 	go func() {
-		closeAllComponents(log, healthService, dataComponents, triesComponents, networkComponents, chanCloseComponents)
+		closeAllComponents(log, healthService, dataComponents, triesComponents, networkComponents, pollingClosers, chanCloseComponents)
 	}()
 
 	_ = tracing.Shutdown() // errors logged internally
@@ -1134,8 +1137,18 @@ func closeAllComponents(
 	dataComponents *factory.DataComponents,
 	triesComponents *factory.TriesComponents,
 	networkComponents *factory.NetworkComponents,
+	pollingClosers []io.Closer,
 	chanCloseComponents chan struct{},
 ) {
+	// Stop polling first so handlers cannot fire against components being torn down below.
+	log.Debug("closing status polling goroutines...")
+	for _, c := range pollingClosers {
+		if c == nil {
+			continue
+		}
+		log.LogIfError(c.Close())
+	}
+
 	log.Debug("closing health service...")
 	err := healthService.Close()
 	log.LogIfError(err)

core/appStatusPolling/appStatusPolling.go

@@ -16,6 +16,8 @@ type AppStatusPolling struct {
 	mutRegisteredFunc   sync.RWMutex
 	registeredFunctions []func(appStatusHandler core.AppStatusHandler)
 	appStatusHandler    core.AppStatusHandler
+	done                chan struct{}
+	closeOnce           sync.Once
 }
 
 // NewAppStatusPolling will return an instance of AppStatusPolling
@@ -29,6 +31,7 @@ func NewAppStatusPolling(appStatusHandler core.AppStatusHandler, pollingDuration
 	return &AppStatusPolling{
 		pollingDuration:  pollingDuration,
 		appStatusHandler: appStatusHandler,
+		done:             make(chan struct{}),
 	}, nil
 }
 
@@ -43,17 +46,32 @@ func (asp *AppStatusPolling) RegisterPollingFunc(handler func(appStatusHandler c
 	return nil
 }
 
-// Poll will notify the AppStatusHandler at a given time
+// Poll will notify the AppStatusHandler at a given time. The goroutine runs
+// until Close is called.
 func (asp *AppStatusPolling) Poll() {
 	go func() {
-		for {
-			time.Sleep(asp.pollingDuration)
+		ticker := time.NewTicker(asp.pollingDuration)
+		defer ticker.Stop()
 
-			asp.mutRegisteredFunc.RLock()
-			for _, handler := range asp.registeredFunctions {
-				handler(asp.appStatusHandler)
+		for {
+			select {
+			case <-asp.done:
+				return
+			case <-ticker.C:
+				asp.mutRegisteredFunc.RLock()
+				for _, handler := range asp.registeredFunctions {
+					handler(asp.appStatusHandler)
+				}
+				asp.mutRegisteredFunc.RUnlock()
 			}
-			asp.mutRegisteredFunc.RUnlock()
 		}
 	}()
 }
+
+// Close stops the polling goroutine. Idempotent; always returns nil.
+func (asp *AppStatusPolling) Close() error {
+	asp.closeOnce.Do(func() {
+		close(asp.done)
+	})
+	return nil
+}