Skip to content

Commit 1a8a496

Browse files
b0bbywanclaude
andcommitted
fix(systemd): keep user unit state in sync at startup and on restart
fsnotify only reports invocation links, so odio-api missed user units that settle without a new one: a unit snapshotted while activating (odio-screen waiting on odio-api), and a restart whose start event landed while the stop was still being watched. Follow the former, and re-read the latter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gFB9WgpMajs1zcfrxyanA
1 parent 3c06737 commit 1a8a496

4 files changed

Lines changed: 163 additions & 7 deletions

File tree

backend/systemd/fsnotify.go

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,53 @@ func (l *Listener) dispatchFSNotify(event fsnotify.Event) {
105105
return
106106
}
107107

108-
if _, loaded := l.watcherMap.LoadOrStore(serviceName, true); !loaded {
109-
go l.waitForStableState(serviceName)
108+
l.track(serviceName)
109+
}
110+
111+
// track starts a watcher for service, or marks the running one: an event it
112+
// has not seen may postdate the state it is about to settle on (a restart).
113+
func (l *Listener) track(service string) {
114+
l.watchMu.Lock()
115+
defer l.watchMu.Unlock()
116+
if l.watching == nil {
117+
l.watching = make(map[string]bool)
118+
}
119+
if _, running := l.watching[service]; running {
120+
l.watching[service] = true
121+
return
110122
}
123+
l.watching[service] = false
124+
go l.waitForStableState(service)
125+
}
126+
127+
// settle ends the watch on service, unless an event came in meanwhile: then it
128+
// clears the mark and reports false, and the watcher reads the state again.
129+
func (l *Listener) settle(service string) bool {
130+
l.watchMu.Lock()
131+
defer l.watchMu.Unlock()
132+
if l.watching[service] {
133+
l.watching[service] = false
134+
return false
135+
}
136+
delete(l.watching, service)
137+
return true
138+
}
139+
140+
func (l *Listener) untrack(service string) {
141+
l.watchMu.Lock()
142+
defer l.watchMu.Unlock()
143+
delete(l.watching, service)
111144
}
112145

113146
func (l *Listener) waitForStableState(service string) {
114147
timeout := l.backend.config.Timeout
115148
ctx, cancel := context.WithTimeout(l.backend.ctx, timeout)
149+
settled := false
116150
defer func() {
117-
l.watcherMap.Delete(service)
151+
// Once settled the entry may already belong to a newer watcher.
152+
if !settled {
153+
l.untrack(service)
154+
}
118155
if ctx.Err() == context.DeadlineExceeded {
119156
logger.Warn("[systemd] %s failed to start in less than %s, cache might be out of sync", service, timeout)
120157
refreshCtx, refreshCancel := context.WithTimeout(l.backend.ctx, 5*time.Second)
@@ -146,8 +183,13 @@ func (l *Listener) waitForStableState(service string) {
146183
timer.Reset(waitTime)
147184
continue
148185
}
149-
switch unit.ActiveState {
150-
case "active", "inactive", "failed":
186+
if isStableState(unit.ActiveState) {
187+
if !l.settle(service) {
188+
logger.Debug("[systemd] %s/%s changed while settling on %s, reading again", ScopeUser, service, unit.ActiveState)
189+
timer.Reset(0)
190+
continue
191+
}
192+
settled = true
151193
logger.Debug("[systemd] %s/%s reached stable state: %s", ScopeUser, service, unit.ActiveState)
152194
l.backend.notifyService(*unit)
153195
return
@@ -162,3 +204,33 @@ func (l *Listener) waitForStableState(service string) {
162204
}
163205
}
164206
}
207+
208+
func isStableState(state string) bool {
209+
switch state {
210+
case "active", "inactive", "failed":
211+
return true
212+
}
213+
return false
214+
}
215+
216+
// trackTransitional follows the watched user units the startup snapshot caught
217+
// mid-transition: their invocation link predates the watcher, so no event comes.
218+
func (l *Listener) trackTransitional(services []Service) {
219+
if l.supportsUTMP {
220+
return // D-Bus signals report the end of the transition
221+
}
222+
for _, name := range transitionalUnits(services, l.userWatched) {
223+
l.track(name)
224+
}
225+
}
226+
227+
// transitionalUnits lists the watched user units not yet in a stable state.
228+
func transitionalUnits(services []Service, watched map[string]bool) []string {
229+
var names []string
230+
for _, svc := range services {
231+
if svc.Scope == ScopeUser && watched[svc.Name] && !isStableState(svc.ActiveState) {
232+
names = append(names, svc.Name)
233+
}
234+
}
235+
return names
236+
}

backend/systemd/systemd.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ func (s *SystemdBackend) Start() error {
7272
logger.Debug("[systemd] starting backend (utmp=%v)", s.config.SupportsUTMP)
7373

7474
// Load the cache at startup
75-
if _, err := s.ListServices(); err != nil {
75+
services, err := s.ListServices()
76+
if err != nil {
7677
return err
7778
}
7879

@@ -81,6 +82,7 @@ func (s *SystemdBackend) Start() error {
8182
if err := s.listener.Start(); err != nil {
8283
return err
8384
}
85+
s.listener.trackTransitional(services)
8486

8587
logger.Info("[systemd] backend started successfully")
8688
return nil
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package systemd
2+
3+
import (
4+
"slices"
5+
"testing"
6+
)
7+
8+
func TestTransitionalUnits(t *testing.T) {
9+
services := []Service{
10+
{Name: "odio-screen.service", Scope: ScopeUser, ActiveState: "activating"},
11+
{Name: "mpd.service", Scope: ScopeUser, ActiveState: "active"},
12+
{Name: "spotifyd.service", Scope: ScopeUser, ActiveState: "failed"},
13+
{Name: "qbzd.service", Scope: ScopeUser, ActiveState: "deactivating"},
14+
{Name: "bluetooth.service", Scope: ScopeSystem, ActiveState: "activating"},
15+
{Name: "unwatched.service", Scope: ScopeUser, ActiveState: "reloading"},
16+
}
17+
watched := map[string]bool{
18+
"odio-screen.service": true,
19+
"mpd.service": true,
20+
"spotifyd.service": true,
21+
"qbzd.service": true,
22+
"bluetooth.service": true,
23+
}
24+
25+
got := transitionalUnits(services, watched)
26+
want := []string{"odio-screen.service", "qbzd.service"}
27+
if !slices.Equal(got, want) {
28+
t.Errorf("transitionalUnits = %v, want %v", got, want)
29+
}
30+
}
31+
32+
func TestTrackTransitionalLeavesDBusModeToSignals(t *testing.T) {
33+
l := &Listener{
34+
supportsUTMP: true,
35+
userWatched: map[string]bool{"odio-screen.service": true},
36+
}
37+
38+
l.trackTransitional([]Service{{Name: "odio-screen.service", Scope: ScopeUser, ActiveState: "activating"}})
39+
40+
if _, tracked := l.watching["odio-screen.service"]; tracked {
41+
t.Error("odio-screen.service tracked in D-Bus mode, want signals to cover it")
42+
}
43+
}
44+
45+
func TestTrackMarksARunningWatch(t *testing.T) {
46+
// A restart: the stop event started the watch, the start event must not be lost.
47+
l := &Listener{watching: map[string]bool{"odio-screen.service": false}}
48+
49+
l.track("odio-screen.service")
50+
51+
if marked := l.watching["odio-screen.service"]; !marked {
52+
t.Error("running watch not marked, the start event would be dropped")
53+
}
54+
}
55+
56+
func TestSettleEndsAQuietWatch(t *testing.T) {
57+
l := &Listener{watching: map[string]bool{"odio-screen.service": false}}
58+
59+
if !l.settle("odio-screen.service") {
60+
t.Fatal("settle = false, want true with no event since")
61+
}
62+
if _, tracked := l.watching["odio-screen.service"]; tracked {
63+
t.Error("settled watch still tracked")
64+
}
65+
}
66+
67+
func TestSettleReadsAgainAfterAnEvent(t *testing.T) {
68+
l := &Listener{watching: map[string]bool{"odio-screen.service": true}}
69+
70+
if l.settle("odio-screen.service") {
71+
t.Fatal("settle = true, want false: an event came in meanwhile")
72+
}
73+
if marked, tracked := l.watching["odio-screen.service"]; !tracked || marked {
74+
t.Errorf("watching = %v, want still tracked with the mark cleared", l.watching)
75+
}
76+
if !l.settle("odio-screen.service") {
77+
t.Error("second settle = false, want true once the event was read")
78+
}
79+
}

backend/systemd/types.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ type Listener struct {
2323
// Deduplication: last known state per service/scope
2424
lastState map[string]string
2525
lastStateMu sync.RWMutex
26-
watcherMap sync.Map
26+
27+
// User units watched to a stable state; true = an event came in since.
28+
watching map[string]bool
29+
watchMu sync.Mutex
2730
}
2831

2932
type UnitScope string

0 commit comments

Comments
 (0)