Stabilize OpenWrt modem startup and upgrades

cmd/vocat/instance_lock_linux.go

@@ -12,19 +12,32 @@ import (
 )
 
 func lockServerInstance(databasePath string) (*os.File, error) {
-	directory := filepath.Dir(databasePath)
+	// The modem, PC/SC reader, XFRM policies and listener are host resources,
-	if err := os.MkdirAll(directory, 0o755); err != nil {
+	// not database resources. Lock per OS user so a diagnostic instance using a
-		return nil, fmt.Errorf("create data directory for instance lock: %w", err)
+	// different VOCAT_DATABASE_PATH cannot silently steal the same AT port from
+	// the managed service. Prefer /run because systemd's PrivateTmp would
+	// otherwise hide the managed service's lock from a manually started process.
+	// The UID-specific directory still permits intentionally isolated users to
+	// operate independently; development hosts without writable /run fall back
+	// to TempDir.
+	uid := os.Geteuid()
+	directory := filepath.Join("/run", fmt.Sprintf("vocat-%d", uid))
+	if uid == 0 {
+		directory = "/run/vocat"
 	}
-	path := filepath.Join(directory, ".vocat.lock")
+	if err := os.MkdirAll(directory, 0o700); err != nil {
-	file, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600)
+		directory = os.TempDir()
+	}
+	path := filepath.Join(directory, "vocat-server.lock")
+	fd, err := unix.Open(path, unix.O_CREAT|unix.O_RDWR|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0o600)
 	if err != nil {
 		return nil, fmt.Errorf("open server instance lock: %w", err)
 	}
-	if err := unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB); err != nil {
+	file := os.NewFile(uintptr(fd), path)
+	if err := unix.Flock(fd, unix.LOCK_EX|unix.LOCK_NB); err != nil {
 		_ = file.Close()
 		if errors.Is(err, unix.EWOULDBLOCK) || errors.Is(err, unix.EAGAIN) {
-			return nil, fmt.Errorf("another vocat server is already using database %s", databasePath)
+			return nil, errors.New("another vocat server already controls this host's modem resources")
 		}
 		return nil, fmt.Errorf("lock server instance: %w", err)
 	}

cmd/vocat/instance_lock_linux_test.go

@@ -9,17 +9,18 @@ import (
 )
 
 func TestServerInstanceLockRejectsSecondProcess(t *testing.T) {
-	database := filepath.Join(t.TempDir(), "vocat.db")
+	firstDatabase := filepath.Join(t.TempDir(), "vocat.db")
-	first, err := lockServerInstance(database)
+	first, err := lockServerInstance(firstDatabase)
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer first.Close()
-	second, err := lockServerInstance(database)
+	secondDatabase := filepath.Join(t.TempDir(), "other.db")
+	second, err := lockServerInstance(secondDatabase)
 	if second != nil {
 		second.Close()
 	}
-	if err == nil || !strings.Contains(err.Error(), "already using database") {
+	if err == nil || !strings.Contains(err.Error(), "already controls this host") {
 		t.Fatalf("second lock error = %v", err)
 	}
 }

cmd/vocat/main.go

@@ -622,12 +622,18 @@ func configureVoWiFiRuntime(
 		}
 		if deviceConfig.VoWiFiEnabled {
 			if entry, mapErr := mapper.Get(deviceConfig.ID); mapErr == nil {
-				flightContext, cancelFlight := context.WithTimeout(ctx, 10*time.Second)
+				flightErr := protectVoWiFiStartupRadio(ctx, deviceManager, entry.ID)
-				_, flightErr := deviceManager.SetFlight(flightContext, entry.ID, true)
-				cancelFlight()
 				if flightErr != nil {
-					_ = manager.Close(context.Background())
+					// A modem can be temporarily unavailable while OpenWrt/procd is
-					return nil, fmt.Errorf("protect device %q before VoWiFi startup: %w", deviceConfig.ID, flightErr)
+					// restarting the service (notably after loading XFRM modules). Do
+					// not take the Web/API service down with it: the orchestrator below
+					// remains fail-closed and its runtime manager retries until CFUN=4
+					// can be established.
+					logger.Warn(
+						"VoWiFi startup radio protection deferred to automatic retry",
+						"device_id", deviceConfig.ID,
+						"error", flightErr,
+					)
 				}
 			}
 			if _, err := manager.RequestEnabled(deviceConfig.ID, true); err != nil {
@@ -639,6 +645,56 @@ func configureVoWiFiRuntime(
 	return manager, nil
 }
 
+const (
+	vowifiStartupRadioAttempts = 3
+	vowifiStartupRadioDelay    = time.Second
+)
+
+type flightModeSetter interface {
+	SetFlight(context.Context, string, bool) (device.FlightResult, error)
+}
+
+func protectVoWiFiStartupRadio(ctx context.Context, manager flightModeSetter, physicalID string) error {
+	return protectVoWiFiStartupRadioWithRetry(
+		ctx,
+		manager,
+		physicalID,
+		vowifiStartupRadioAttempts,
+		vowifiStartupRadioDelay,
+	)
+}
+
+func protectVoWiFiStartupRadioWithRetry(
+	ctx context.Context,
+	manager flightModeSetter,
+	physicalID string,
+	attempts int,
+	delay time.Duration,
+) error {
+	var lastErr error
+	for attempt := 0; attempt < attempts; attempt++ {
+		flightContext, cancel := context.WithTimeout(ctx, 10*time.Second)
+		_, lastErr = manager.SetFlight(flightContext, physicalID, true)
+		cancel()
+		if lastErr == nil {
+			return nil
+		}
+		if attempt+1 == attempts {
+			break
+		}
+		timer := time.NewTimer(delay)
+		select {
+		case <-ctx.Done():
+			if !timer.Stop() {
+				<-timer.C
+			}
+			return errors.Join(lastErr, ctx.Err())
+		case <-timer.C:
+		}
+	}
+	return lastErr
+}
+
 type vowifiDeviceAdapter interface {
 	vowifi.SIMIdentityReader
 	vowifi.AKAProvider

cmd/vocat/vowifi_startup_test.go

@@ -0,0 +1,56 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"vocat/internal/device"
+)
+
+type startupFlightSetter struct {
+	errors []error
+	calls  int
+	id     string
+}
+
+func (setter *startupFlightSetter) SetFlight(
+	_ context.Context,
+	id string,
+	enabled bool,
+) (device.FlightResult, error) {
+	setter.calls++
+	setter.id = id
+	if !enabled {
+		return device.FlightResult{}, errors.New("expected flight mode to be enabled")
+	}
+	if setter.calls <= len(setter.errors) {
+		return device.FlightResult{}, setter.errors[setter.calls-1]
+	}
+	return device.FlightResult{CurrentMode: 4, FlightMode: true, RadioOff: true}, nil
+}
+
+func TestProtectVoWiFiStartupRadioRetriesTransientFailure(t *testing.T) {
+	transient := errors.New("modem is reopening")
+	setter := &startupFlightSetter{errors: []error{transient, transient}}
+	if err := protectVoWiFiStartupRadioWithRetry(
+		context.Background(), setter, "quectel-1", 3, 0,
+	); err != nil {
+		t.Fatalf("protect startup radio: %v", err)
+	}
+	if setter.calls != 3 || setter.id != "quectel-1" {
+		t.Fatalf("SetFlight calls = %d, id = %q", setter.calls, setter.id)
+	}
+}
+
+func TestProtectVoWiFiStartupRadioReturnsLastFailure(t *testing.T) {
+	first := errors.New("first")
+	last := errors.New("last")
+	setter := &startupFlightSetter{errors: []error{first, last}}
+	err := protectVoWiFiStartupRadioWithRetry(
+		context.Background(), setter, "quectel-1", 2, 0,
+	)
+	if !errors.Is(err, last) || setter.calls != 2 {
+		t.Fatalf("protect startup radio = %v after %d calls", err, setter.calls)
+	}
+}

scripts/install.sh

@@ -337,6 +337,8 @@ TimeoutStartSec=30s
 # HTTP, VoWiFi, and modem cleanup have bounded shutdown contexts totalling up
 # to 30 seconds. Leave a small margin before systemd resorts to SIGKILL.
 TimeoutStopSec=40s
+RuntimeDirectory=vocat
+RuntimeDirectoryMode=0755
 
 AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW
 CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW
@@ -409,11 +411,24 @@ enable_and_start() {
     if [ -x "$OPENWRT_INIT_PATH" ] && { [ -x /sbin/procd ] || [ -x /sbin/ubusd ]; }; then
         "$OPENWRT_INIT_PATH" enable
         if "$OPENWRT_INIT_PATH" restart; then
-            sleep 2
+            # Modems may need several seconds to release and reopen their AT
+            # port after procd stops the previous process. Require consecutive
+            # healthy observations so a short-lived respawn is not mistaken for
+            # a successful upgrade.
+            local attempt stable
+            stable=0
+            for attempt in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30; do
+                sleep 1
                 if "$OPENWRT_INIT_PATH" running; then
+                    stable=$((stable + 1))
+                    if [ "$stable" -ge 3 ]; then
                         rm -f "${BINARY_PATH}.bak"
                         return
                     fi
+                else
+                    stable=0
+                fi
+            done
         fi
         if [ -e "${BINARY_PATH}.bak" ]; then
             cp -a "${BINARY_PATH}.bak" "$BINARY_PATH"