Skip to content

Commit eda0185

Browse files
fix(cache): refresh serve daemon's cache from disk each render cycle (#7774)
* fix(cache): refresh serve daemon's cache from disk each render cycle The `serve` daemon (used by streaming mode) loads the on-disk Session and Device caches once at startup and only flushes them back on exit, so a write from a separate one-shot process was invisible to the running daemon until it restarted, and could even be clobbered by the daemon's own stale copy on shutdown. This affected `oh-my-posh toggle` (Session store) as well as `enable`/`disable` - e.g. `enable reload`, which config.Get in prompt.New checks to bypass its own config cache after an edit (Device store). Fixes #7758. Add cache.Refresh(), which re-syncs the in-memory store from disk when the file's mtime has advanced, merging entries by Timestamp so a value the daemon has itself set more recently than the file always wins. Call it for both stores at the start of every render cycle in serve.go, and once more before close() persists a dirty store, closing the shutdown clobber window too. Drop the Session-only guard on the mtime bump in store.close() so a Device write reliably updates the file's mtime on Windows as well (the mmap-backed write path doesn't do this on its own). Also documents the residual limitation (there's still a narrow window where a write can land mid-cycle) in the streaming docs. * fix(cache): reorder store fields to satisfy fieldalignment Adding mtime widened the struct's pointer-scannable prefix from 40 to 56 bytes (the time.Time's trailing *Location pointer landed after the three trailing bools). Reorder so pointer-bearing fields lead and the bools trail, matching what CI's fieldalignment check expects. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e446fdc commit eda0185

4 files changed

Lines changed: 277 additions & 9 deletions

File tree

src/cache/store.go

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ import (
1414
)
1515

1616
type store struct {
17+
// mtime is the on-disk file's modification time as of the last load or
18+
// refresh. A long-lived process (serve) uses it to detect writes made by
19+
// other processes (e.g. toggle) between render cycles - see Refresh.
20+
mtime time.Time
1721
cache *maps.Concurrent[*Entry[any]]
1822
filePath string
1923
dirty bool
@@ -72,6 +76,7 @@ func (s Store) init(filePath string, persist bool) {
7276
store.persist = persist
7377
store.dirty = false
7478
store.locked = false
79+
store.mtime = time.Time{}
7580

7681
reader, err := openFile(store.filePath)
7782
if err != nil {
@@ -92,6 +97,10 @@ func (s Store) init(filePath string, persist bool) {
9297

9398
defer reader.Close()
9499

100+
if info, err := os.Stat(store.filePath); err == nil {
101+
store.mtime = info.ModTime()
102+
}
103+
95104
var list maps.Simple[*Entry[any]]
96105

97106
dec := gob.NewDecoder(reader)
@@ -132,10 +141,70 @@ func touchSessionFile(filePath string) {
132141
}
133142
}
134143

144+
// Refresh re-syncs the in-memory store with the on-disk file if it has
145+
// changed since the last load or refresh, merging entries by Timestamp (the
146+
// newer value wins). A short-lived, one-shot invocation never needs this -
147+
// init() already loads the current file once. It exists for a long-lived
148+
// process (serve) that keeps its cache in memory for the session: without
149+
// it, a write from another process (e.g. `toggle`, `enable`/`disable`)
150+
// would stay invisible until the daemon exits.
151+
func Refresh(s Store) {
152+
defer log.Trace(time.Now(), string(s))
153+
154+
store := s.get()
155+
if store == nil || store.locked || store.filePath == "" {
156+
return
157+
}
158+
159+
info, err := os.Stat(store.filePath)
160+
if err != nil || !info.ModTime().After(store.mtime) {
161+
return
162+
}
163+
164+
reader, err := openFile(store.filePath)
165+
if err != nil {
166+
return
167+
}
168+
169+
defer reader.Close()
170+
171+
var list maps.Simple[*Entry[any]]
172+
173+
dec := gob.NewDecoder(reader)
174+
if err := dec.Decode(&list); err != nil {
175+
log.Error(err)
176+
return
177+
}
178+
179+
for key, diskEntry := range list {
180+
if diskEntry.Expired() {
181+
continue
182+
}
183+
184+
if current, found := store.cache.Get(key); found && current.Timestamp >= diskEntry.Timestamp {
185+
continue
186+
}
187+
188+
log.Debugf("(%s) refreshing %s from disk", string(s), key)
189+
store.cache.Set(key, diskEntry)
190+
}
191+
192+
store.mtime = info.ModTime()
193+
}
194+
135195
func (s Store) close() {
136196
defer log.Trace(time.Now(), string(s))
137197

138198
store := s.get()
199+
200+
// Pick up any write from another process one last time before a dirty
201+
// store overwrites the file, so a change made after the last Refresh
202+
// (e.g. right before the shell exits) isn't clobbered by this store's
203+
// own, possibly stale, in-memory copy.
204+
if store != nil && !store.locked && store.persist && store.dirty {
205+
Refresh(s)
206+
}
207+
139208
if store == nil || store.locked || !store.persist || !store.dirty {
140209
if s == Session && store != nil && !store.locked && store.filePath != "" {
141210
touchSessionFile(store.filePath)
@@ -169,15 +238,13 @@ func (s Store) close() {
169238
log.Error(err)
170239
}
171240

172-
if s != Session {
173-
return
174-
}
175-
176241
// On Windows, the mmap-backed write path doesn't reliably update the
177-
// file's on-disk last-write-time (per Microsoft's docs), which can lead
178-
// to an actively-used session cache being mistaken for stale and swept
179-
// up by cache.Clear(). Explicitly bump the mtime now that the file is
180-
// closed (and the mmap unmap/flush on Windows has happened).
242+
// file's on-disk last-write-time (per Microsoft's docs). For the session
243+
// store that can lead to an actively-used cache being mistaken for stale
244+
// and swept up by cache.Clear(); for either store, a long-lived Refresh
245+
// reader (serve) needs a trustworthy mtime to notice this write at all.
246+
// Explicitly bump it now that the file is closed (and the mmap
247+
// unmap/flush on Windows has happened).
181248
if err := os.Chtimes(store.filePath, time.Now(), time.Now()); err != nil {
182249
log.Error(err)
183250
}

src/cache/store_test.go

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"testing"
99
"time"
1010

11+
"github.com/jandedobbeleer/oh-my-posh/src/maps"
12+
1113
"github.com/stretchr/testify/assert"
1214
"github.com/stretchr/testify/require"
1315
)
@@ -164,3 +166,181 @@ func TestGetSurvivesGobPointerRegistration(t *testing.T) {
164166
require.True(t, ok, "expected a cache hit after a gob round-trip of a pointer-registered type")
165167
assert.Equal(t, "value", got.Name)
166168
}
169+
170+
// Guards against the #7758 class of bug: a long-lived process (serve) that
171+
// loaded the session cache once at startup must still see a write from a
172+
// separate one-shot process (e.g. `oh-my-posh toggle`) that landed on disk
173+
// afterwards, without losing values it has itself set more recently than
174+
// what's on disk (e.g. prompt_count_cache).
175+
func TestRefreshMergesExternalWriteByTimestamp(t *testing.T) {
176+
origSession := session
177+
t.Cleanup(func() { session = origSession })
178+
179+
filePath := filepath.Join(t.TempDir(), "session.cache")
180+
181+
// The daemon's in-memory state: it has its own, newer value for a key
182+
// the external writer also touches, and predates the external write.
183+
daemon := Session.new()
184+
daemon.filePath = filePath
185+
daemon.persist = true
186+
daemon.mtime = time.Now().Add(-time.Hour)
187+
daemon.cache.Set("shared_key", &Entry[any]{
188+
Value: "daemon-value",
189+
Timestamp: time.Now().Unix(),
190+
TTL: -1,
191+
})
192+
193+
// Simulate `oh-my-posh toggle` running as a separate one-shot process:
194+
// it starts from a stale copy of shared_key and writes a brand new key.
195+
writer := Session.new()
196+
writer.filePath = filePath
197+
writer.persist = true
198+
writer.dirty = true
199+
writer.cache.Set("shared_key", &Entry[any]{
200+
Value: "stale-external-copy",
201+
Timestamp: time.Now().Unix() - 100,
202+
TTL: -1,
203+
})
204+
writer.cache.Set(TOGGLECACHE, &Entry[any]{
205+
Value: map[string]bool{"shell": true},
206+
Timestamp: time.Now().Unix(),
207+
TTL: -1,
208+
})
209+
session = writer
210+
Session.close()
211+
212+
// Back to the daemon's perspective: refresh should pick up the new
213+
// toggle_cache key from disk...
214+
session = daemon
215+
Refresh(Session)
216+
217+
toggled, found := Get[map[string]bool](Session, TOGGLECACHE)
218+
require.True(t, found, "toggle_cache written externally should be visible after Refresh")
219+
assert.True(t, toggled["shell"])
220+
221+
// ...without clobbering the daemon's own newer value for a key both
222+
// sides touched.
223+
shared, found := Get[string](Session, "shared_key")
224+
require.True(t, found)
225+
assert.Equal(t, "daemon-value", shared, "a newer in-memory value must win over an older on-disk one")
226+
}
227+
228+
// Guards against the daemon's own shutdown flush clobbering a write that
229+
// landed after its last Refresh but before it exits.
230+
func TestCloseRefreshesBeforePersistingToAvoidClobber(t *testing.T) {
231+
origSession := session
232+
t.Cleanup(func() { session = origSession })
233+
234+
filePath := filepath.Join(t.TempDir(), "session.cache")
235+
236+
daemon := Session.new()
237+
daemon.filePath = filePath
238+
daemon.persist = true
239+
daemon.dirty = true
240+
daemon.mtime = time.Now().Add(-time.Hour)
241+
daemon.cache.Set("prompt_count_cache", &Entry[any]{
242+
Value: 3,
243+
Timestamp: time.Now().Unix(),
244+
TTL: -1,
245+
})
246+
247+
// A toggle write lands on disk after the daemon's last refresh, right
248+
// before the shell (and daemon) exit.
249+
writer := Session.new()
250+
writer.filePath = filePath
251+
writer.persist = true
252+
writer.dirty = true
253+
writer.cache.Set(TOGGLECACHE, &Entry[any]{
254+
Value: map[string]bool{"shell": true},
255+
Timestamp: time.Now().Unix(),
256+
TTL: -1,
257+
})
258+
session = writer
259+
Session.close()
260+
261+
// The daemon now shuts down. Its close() must merge the external write
262+
// in before overwriting the file, not blindly persist its stale copy.
263+
session = daemon
264+
Session.close()
265+
266+
reader, err := openFile(filePath)
267+
require.NoError(t, err)
268+
defer reader.Close()
269+
270+
var onDisk maps.Simple[*Entry[any]]
271+
require.NoError(t, gob.NewDecoder(reader).Decode(&onDisk))
272+
273+
session = &store{cache: onDisk.ToConcurrent()}
274+
275+
toggled, found := Get[map[string]bool](Session, TOGGLECACHE)
276+
require.True(t, found, "the external write must survive the daemon's own shutdown flush")
277+
assert.True(t, toggled["shell"])
278+
279+
count, found := Get[int](Session, "prompt_count_cache")
280+
require.True(t, found)
281+
assert.Equal(t, 3, count)
282+
}
283+
284+
// Guards against a variant of #7758: the serve daemon's Device store (which
285+
// holds the RELOAD flag config.Get checks in prompt.New to bypass its own
286+
// config cache) must also see a write from a separate `oh-my-posh enable
287+
// reload` process, not just the Session store.
288+
func TestRefreshPicksUpDeviceStoreWrite(t *testing.T) {
289+
origDevice := device
290+
t.Cleanup(func() { device = origDevice })
291+
292+
filePath := filepath.Join(t.TempDir(), "omp.cache")
293+
294+
daemon := Device.new()
295+
daemon.filePath = filePath
296+
daemon.persist = true
297+
daemon.mtime = time.Now().Add(-time.Hour)
298+
device = daemon
299+
300+
writer := Device.new()
301+
writer.filePath = filePath
302+
writer.persist = true
303+
writer.dirty = true
304+
writer.cache.Set("reload", &Entry[any]{
305+
Value: true,
306+
Timestamp: time.Now().Unix(),
307+
TTL: -1,
308+
})
309+
device = writer
310+
Device.close()
311+
312+
device = daemon
313+
Refresh(Device)
314+
315+
reload, found := Get[bool](Device, "reload")
316+
require.True(t, found, "`enable reload` written externally should be visible after Refresh")
317+
assert.True(t, reload)
318+
}
319+
320+
// Device's close() must bump the file's mtime as reliably as Session's does
321+
// (the mmap-backed Windows write path doesn't do this on its own) - without
322+
// it, Refresh would never notice an `enable`/`disable` write on Windows.
323+
func TestStoreCloseTouchesDeviceFileMTime(t *testing.T) {
324+
origDevice := device
325+
t.Cleanup(func() { device = origDevice })
326+
327+
filePath := filepath.Join(t.TempDir(), "omp.cache")
328+
329+
testStore := Device.new()
330+
testStore.filePath = filePath
331+
testStore.persist = true
332+
testStore.dirty = true
333+
testStore.cache.Set("reload", &Entry[any]{
334+
Value: true,
335+
Timestamp: time.Now().Unix(),
336+
TTL: -1,
337+
})
338+
device = testStore
339+
340+
before := time.Now()
341+
Device.close()
342+
343+
info, err := os.Stat(filePath)
344+
require.NoError(t, err)
345+
assert.False(t, info.ModTime().Before(before), "mtime should be bumped to the close time, not left stale")
346+
}

src/cli/serve.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,18 @@ func startRenderCycle(req *serveRequest, out *os.File, envKeys map[string]struct
266266
}
267267
}()
268268

269+
// The daemon keeps its caches in memory for its whole lifetime (see the
270+
// comment on copyRecords below) and only reads the on-disk files once,
271+
// at startup. Refresh picks up writes from other processes that landed
272+
// since the last cycle: Session for `oh-my-posh toggle` (a segment
273+
// toggled mid-session shouldn't stay stuck until the daemon exits), and
274+
// Device for `enable`/`disable` (e.g. `enable reload`, which config.Get
275+
// in prompt.New checks to bypass its own config cache after an edit -
276+
// without this the daemon would keep serving the old config until
277+
// restarted).
278+
cache.Refresh(cache.Session)
279+
cache.Refresh(cache.Device)
280+
269281
// Apply the env overlay BEFORE constructing the engine so segment
270282
// execution and config templates observe the calling shell's
271283
// environment. v1 accepts the theoretical race with a still-running
@@ -397,7 +409,9 @@ func copyRecords(id int64, records <-chan string, out *os.File) chan struct{} {
397409
// template caches in memory for the daemon's lifetime - that's the
398410
// whole point of a long-lived process. Caches are only flushed to
399411
// disk once, on clean shutdown (quit/EOF), via the cache.Close()/
400-
// template.SaveCache() defer in createServeCmd.
412+
// template.SaveCache() defer in createServeCmd. Reads are a
413+
// different story: cache.Refresh() in startRenderCycle re-syncs from
414+
// disk each cycle, so writes from other processes are still seen.
401415
}()
402416

403417
return done

website/docs/configuration/streaming.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,13 @@ Requires [Clink][clink] v1.1.42 or later.
117117
</TabItem>
118118
</Tabs>
119119

120+
## Known limitations
121+
122+
The background process re-syncs its in-memory cache from disk before every render, so writes
123+
from another process (`toggle`, `enable`/`disable`) are picked up on the next prompt. There can
124+
still be a narrow window - a write that lands between the background process reading the cache
125+
and finishing that same render - where it takes one extra prompt to show up.
126+
120127
## Feedback
121128

122129
If you encounter issues or have suggestions for the streaming feature, please open an issue on the

0 commit comments

Comments
 (0)