fix(control): wait for SPN to fully stop before completing pause operation

service/control/module.go

@@ -26,11 +26,14 @@ type Control struct {
 	resumeWorker      *mgr.WorkerMgr
 	pauseNotification *notifications.Notification
 	pauseInfo         PauseInfo
+
+	cfgSpnEnabled config.BoolOption
 }
 
 type instance interface {
 	Config() *config.Config
 	InterceptionGroup() *mgr.GroupModule
+	SPNGroup() *mgr.ExtendedGroup
 	IsShuttingDown() bool
 }
 
@@ -45,9 +48,10 @@ func New(instance instance) (*Control, error) {
 
 	m := mgr.New("Control")
 	module := &Control{
-		mgr:      m,
+		mgr:           m,
-		instance: instance,
+		instance:      instance,
-		states:   mgr.NewStateMgr(m),
+		states:        mgr.NewStateMgr(m),
+		cfgSpnEnabled: config.GetAsBool("spn/enable", false),
 	}
 	if err := module.prep(); err != nil {
 		return nil, err

service/control/pause.go

@@ -32,13 +32,12 @@ func (c *Control) pause(duration time.Duration, onlySPN bool) (retErr error) {
 		return errors.New("invalid pause duration")
 	}
 
-	spn_enabled := config.GetAsBool("spn/enable", false)
 	if onlySPN {
 		if c.pauseInfo.Interception {
 			return errors.New("cannot pause SPN separately when core is paused")
 		}
 		// If SPN is not running and not already paused, cannot pause it or change pause duration.
-		if !spn_enabled() && !c.pauseInfo.SPN {
+		if !c.cfgSpnEnabled() && !c.pauseInfo.SPN {
 			return errors.New("cannot pause SPN when it is not running")
 		}
 	}
@@ -54,11 +53,21 @@ func (c *Control) pause(duration time.Duration, onlySPN bool) (retErr error) {
 
 	// Pause SPN if not already paused.
 	if !c.pauseInfo.SPN {
-		if spn_enabled() {
+		if c.cfgSpnEnabled() {
+			// "spn/access" module is responsible for starting/stopping SPN service.
+			// Here we just change the config to notify it to stop SPN.
 			// TODO: the 'pause' state must not make permanent config changes.
-			// Consider possibility to not store permanent config changes.
+			//       Consider possibility to not store permanent config changes.
-			// E.g. SPN enabled -> pause SPN -> restart PC/Portmaster -> SPN should be enabled again.
+			//       E.g. SPN enabled -> pause SPN -> restart PC/Portmaster -> SPN should be enabled again.
 			config.SetConfigOption("spn/enable", false)
+
+			// Wait until SPN is fully stopped with timeout 30s.
+			err := c.waitSPNStopped(time.Second * 30)
+			if err != nil {
+				config.SetConfigOption("spn/enable", true) // revert config change on error
+				return err
+			}
+
 			c.mgr.Info("SPN paused")
 			c.pauseInfo.SPN = true
 		}
@@ -108,8 +117,9 @@ func (c *Control) resume() (retErr error) {
 	}
 
 	if c.pauseInfo.SPN {
-		enabled := config.GetAsBool("spn/enable", false)
+		// "spn/access" module is responsible for starting/stopping SPN service.
-		if !enabled() {
+		// Here we just change the config to notify it to start SPN.
+		if !c.cfgSpnEnabled() {
 			config.SetConfigOption("spn/enable", true)
 			c.mgr.Info("SPN resumed")
 		}
@@ -157,8 +167,8 @@ func (c *Control) startResumeWorker(duration time.Duration) {
 			case <-wc.Ctx().Done():
 				return nil
 			case <-cfgChangeEvt.Events():
-				spnEnabled := config.GetAsBool("spn/enable", false)
+				if c.cfgSpnEnabled() {
-				if spnEnabled() {
+					cfgChangeEvt.Cancel() // we do not need it anymore (no problem to cancel multiple times)
 					wc.Info("SPN enabled by user. Auto-resume initiated.")
 					needToAutoResume = true
 				}
@@ -266,3 +276,50 @@ func (c *Control) updateStatesAndNotifyError(errDescription string, err error) {
 	notifications.Notify(c.pauseNotification)
 	c.pauseNotification.SyncWithState(c.states)
 }
+
+func (c *Control) showNotification(title, message string) *notifications.Notification {
+	n := &notifications.Notification{
+		EventID: "control:status_info",
+		Type:    notifications.Info,
+		Title:   title,
+		Message: message,
+	}
+	notifications.Notify(n)
+	return n
+}
+
+func (c *Control) waitSPNStopped(stopTimeout time.Duration) error {
+	var notification *notifications.Notification
+	defer func() {
+		if notification != nil {
+			notification.Delete()
+		}
+	}()
+
+	startTime := time.Now()
+	isStopped, _ := c.instance.SPNGroup().IsStopped()
+	for !isStopped {
+		var err error
+
+		time.Sleep(200 * time.Millisecond)
+
+		if c.mgr.IsDone() || c.instance.IsShuttingDown() {
+			return errors.New("shutting down")
+		}
+
+		isStopped, err = c.instance.SPNGroup().IsStopped()
+		if err != nil {
+			return fmt.Errorf("failed to stop SPN: %w", err)
+		}
+		if time.Since(startTime) > stopTimeout {
+			return errors.New("timeout waiting for SPN to stop")
+		}
+		if notification == nil && time.Since(startTime) > time.Second {
+			notification = c.showNotification("Waiting for SPN to stop...", "")
+		}
+		if c.cfgSpnEnabled() {
+			return errors.New("SPN enabled again")
+		}
+	}
+	return nil
+}

service/mgr/group.go

@@ -159,6 +159,16 @@ func (g *Group) Start() error {
 	return nil
 }
 
+// IsStopped returns whether the group is stopped.
+// It returns an error if the group is in an invalid state.
+func (g *Group) IsStopped() (bool, error) {
+	state := g.state.Load()
+	if state == groupStateInvalid {
+		return false, errors.New("invalid group state")
+	}
+	return g.state.Load() == groupStateOff, nil
+}
+
 // Stop stops all modules in the group in the reverse order.
 func (g *Group) Stop() error {
 	// Check group state.