From 4fc590e1d329aa9e05e64ae3a15d91481aa86e1d Mon Sep 17 00:00:00 2001
From: Zygmunt Krynicki <zygmunt.krynicki@huawei.com>
Date: Thu, 31 Mar 2022 21:05:05 +0200
Subject: [PATCH] Never exit when idle but reboot is pending

The previous approach for making reboot pending postpone service
shutdown was not robust, because it would only extend the interval of
the check, not stop the actual exit.

It worked fine for shorter periods, as the default (60 second) value was
masking what the integration tests were measuring (10 seconds).

Extend RebootDelayer interface with a new proto.RebootPending method
which checks if a reboot timer exists. This makes the check race-free
regardless of the frequency.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@huawei.com>
---
 boot/boot.go           |  1 +
 boot/grub/grub.go      | 25 +++++++++++++++++++++++--
 boot/piboot/piboot.go  | 41 ++++++++++++++++++++++++++++++++---------
 cmd/sysotad/service.go | 10 +++++++++-
 cmd/sysotad/sysotad.go |  6 ------
 5 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/boot/boot.go b/boot/boot.go
index cfd17d4..0e65afa 100644
--- a/boot/boot.go
+++ b/boot/boot.go
@@ -35,6 +35,7 @@ type Protocol interface {
 // RebootDelayer allows boot protocol to indicate support for postponing the reboot process.
 type RebootDelayer interface {
 	SetRebootDelay(time.Duration)
+	RebootPending() bool
 }
 
 // invalidString is the used by String to represent the invalid state of enumerations.
diff --git a/boot/grub/grub.go b/boot/grub/grub.go
index a358634..771c880 100644
--- a/boot/grub/grub.go
+++ b/boot/grub/grub.go
@@ -28,6 +28,7 @@ package grub
 import (
 	"errors"
 	"fmt"
+	"sync"
 	"time"
 
 	"gitlab.com/zygoon/go-grub"
@@ -68,8 +69,11 @@ var (
 
 // Boot implements boot.Protocol for the GRUB bootloader.
 type Boot struct {
-	envPath     string
+	envPath string
+
+	m           sync.Mutex
 	rebootDelay time.Duration
+	rebootTimer *time.Timer
 }
 
 // New creates a new GrubBoot operating on the given GRUB environment file.
@@ -83,9 +87,20 @@ func New(envPath string) *Boot {
 
 // SetRebootDelay causes the post-install handler to wait before rebooting.
 func (b *Boot) SetRebootDelay(d time.Duration) {
+	b.m.Lock()
+	defer b.m.Unlock()
+
 	b.rebootDelay = d
 }
 
+// RebootPending returns true if a reboot is pending.
+func (b *Boot) RebootPending() bool {
+	b.m.Lock()
+	defer b.m.Unlock()
+
+	return b.rebootTimer != nil
+}
+
 // QueryActive returns the slot that is used for booting.
 //
 // Active slot is determined by the value of the SYSOTA_BOOT_ACTIVE variable.
@@ -204,15 +219,21 @@ func (b *Boot) PreInstall(env *installhandler.Environment) (err error) {
 
 // PostInstall reboots the system.
 func (b *Boot) PostInstall(env *installhandler.Environment) error {
+	b.m.Lock()
+
 	if b.rebootDelay == 0 {
+		b.m.Unlock()
+
 		return b.Reboot(0)
 	}
 
 	// NOTE: this interacts with the exit-when-inactive feature.
-	_ = time.AfterFunc(b.rebootDelay, func() {
+	b.rebootTimer = time.AfterFunc(b.rebootDelay, func() {
 		_ = b.Reboot(0)
 	})
 
+	b.m.Unlock()
+
 	return nil
 }
 
diff --git a/boot/piboot/piboot.go b/boot/piboot/piboot.go
index 89fa986..61d4d0e 100644
--- a/boot/piboot/piboot.go
+++ b/boot/piboot/piboot.go
@@ -26,6 +26,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"gitlab.com/zygoon/go-raspi/pieeprom"
@@ -62,7 +63,10 @@ type PiBoot struct {
 	procMountPoint string
 	revCode        pimodel.RevisionCode
 	serialNum      pimodel.SerialNumber
-	rebootDelay    time.Duration
+
+	m           sync.Mutex
+	rebootDelay time.Duration
+	rebootTimer *time.Timer
 }
 
 // New returns a new PiBoot with the given boot mount point and CPU properties.
@@ -78,9 +82,20 @@ func New(bootMountPoint, procMountPoint string, revCode pimodel.RevisionCode, se
 
 // SetRebootDelay causes the post-install handler to wait before rebooting.
 func (b *PiBoot) SetRebootDelay(d time.Duration) {
+	b.m.Lock()
+	defer b.m.Unlock()
+
 	b.rebootDelay = d
 }
 
+// RebootPending returns true if a reboot is pending.
+func (b *PiBoot) RebootPending() bool {
+	b.m.Lock()
+	defer b.m.Unlock()
+
+	return b.rebootTimer != nil
+}
+
 // BootMountPoint returns the corresponding value passed to the New.
 func (pi *PiBoot) BootMountPoint() string {
 	return pi.bootMountPoint
@@ -365,17 +380,25 @@ func (pi *PiBoot) PreInstall(env *installhandler.Environment) (err error) {
 // Reboots are required after writing to RAUC slot of type "system".
 // Reboot is graceful, shutting down services normally.
 func (pi *PiBoot) PostInstall(env *installhandler.Environment) error {
-	if shouldReboot(env) {
-		if pi.rebootDelay == 0 {
-			return pi.Reboot(boot.RebootTryBoot)
-		}
+	if !shouldReboot(env) {
+		return nil
+	}
+
+	pi.m.Lock()
 
-		// NOTE: this interacts with the exit-when-inactive feature.
-		_ = time.AfterFunc(pi.rebootDelay, func() {
-			_ = pi.Reboot(boot.RebootTryBoot)
-		})
+	if pi.rebootDelay == 0 {
+		pi.m.Unlock()
+
+		return pi.Reboot(boot.RebootTryBoot)
 	}
 
+	// NOTE: this interacts with the exit-when-inactive feature.
+	pi.rebootTimer = time.AfterFunc(pi.rebootDelay, func() {
+		_ = pi.Reboot(boot.RebootTryBoot)
+	})
+
+	pi.m.Unlock()
+
 	return nil
 }
 
diff --git a/cmd/sysotad/service.go b/cmd/sysotad/service.go
index fd9d370..e759811 100644
--- a/cmd/sysotad/service.go
+++ b/cmd/sysotad/service.go
@@ -81,7 +81,8 @@ type Service struct {
 	stateGuard ota.StateGuard
 	confGuard  ota.ConfigGuard
 
-	update updatehosted.HostedService
+	update    updatehosted.HostedService
+	bootProto boot.Protocol
 
 	test testhosted.HostedService
 	exit chan<- struct{}
@@ -144,6 +145,8 @@ func NewService(conn *dbus.Conn, opts ...Option) (*Service, error) {
 		return nil, err
 	}
 
+	svc.bootProto = bootProto
+
 	// Ask the boot protocol to delay the reboot process if this is supported.
 	if delayer, ok := bootProto.(boot.RebootDelayer); ok {
 		delayer.SetRebootDelay(time.Duration(svc.confGuard.Config().QuirksRebootDelay) * time.Second)
@@ -233,5 +236,10 @@ func (svc *Service) SetBootMode(bootMode boot.Mode, isRollback bool) error {
 
 // IsIdle returns true if all the hosted service are idle.
 func (svc *Service) IsIdle() bool {
+	// Pending reboot always postpones service shutdown.
+	if proto, ok := svc.bootProto.(boot.RebootDelayer); ok && proto.RebootPending() {
+		return false
+	}
+
 	return svc.update.IsIdle()
 }
diff --git a/cmd/sysotad/sysotad.go b/cmd/sysotad/sysotad.go
index 849bf03..a3abe61 100644
--- a/cmd/sysotad/sysotad.go
+++ b/cmd/sysotad/sysotad.go
@@ -76,12 +76,6 @@ func (cmd *Cmd) Run(ctx context.Context, _ []string) (err error) {
 		return err
 	}
 
-	// When reboot quirk is enabled, prolong the lifecycle of the service to
-	// allow it to stick around long enough to reboot the system.
-	if d := svc.confGuard.Config().QuirksRebootDelay; d > 0 {
-		cmd.Opts.IdleDuration += time.Duration(d) * time.Second
-	}
-
 	// Export the service to D-Bus having finished configuration.
 	if err := svc.Export(); err != nil {
 		return err
-- 
GitLab