fix(diary): retire replaced daily writers and serialise ApplyGlobals
- install and Reset retire the channels they replace: a retired daily writer appends a late record and closes the file again, so no record is lost and no descriptor stays open; diary.Close keeps reopening the installed writers on the next record - ApplyGlobals holds the setup lock for its whole run (lock-free installLocked and setVerbosityLocked), so concurrent calls rebuild the channels once (WR-06)
This commit is contained in:
@@ -40,6 +40,10 @@ type dailyWriter struct {
|
||||
retryAt time.Time
|
||||
// warnedDate is the last date a failure was reported for.
|
||||
warnedDate string
|
||||
// retired is set when a new setup replaced the writer's channels: a
|
||||
// late record is still appended, but the file is closed again after
|
||||
// every write, so the writer keeps no descriptor.
|
||||
retired bool
|
||||
}
|
||||
|
||||
func newDailyWriter(path string, days int, now func() time.Time, warn func(string, error)) *dailyWriter {
|
||||
@@ -88,6 +92,10 @@ func (d *dailyWriter) Write(p []byte) (int, error) {
|
||||
d.failedDate = ""
|
||||
}
|
||||
n, err := d.file.Write(p)
|
||||
if d.retired {
|
||||
_ = d.file.Close()
|
||||
d.file, d.date = nil, ""
|
||||
}
|
||||
d.mu.Unlock()
|
||||
return n, err
|
||||
}
|
||||
@@ -155,6 +163,16 @@ func (d *dailyWriter) prune(today string) {
|
||||
}
|
||||
}
|
||||
|
||||
// retire closes the open file for good: later writes, from goroutines that
|
||||
// still hold the replaced channels, open the day's file, append and close
|
||||
// it again.
|
||||
func (d *dailyWriter) retire() error {
|
||||
d.mu.Lock()
|
||||
d.retired = true
|
||||
d.mu.Unlock()
|
||||
return d.Close()
|
||||
}
|
||||
|
||||
// Close closes the open file. A later write opens it again.
|
||||
func (d *dailyWriter) Close() error {
|
||||
d.mu.Lock()
|
||||
|
||||
@@ -156,6 +156,12 @@ func Setup(cfg *compass.Config, opts Options) error {
|
||||
func install(conf Config, ch *channels, opts Options) {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
installLocked(conf, ch, opts)
|
||||
}
|
||||
|
||||
// installLocked makes ch the installed setup and retires the channels it
|
||||
// replaces; the caller holds mu.
|
||||
func installLocked(conf Config, ch *channels, opts Options) {
|
||||
if saved == nil {
|
||||
saved = &savedDefault{logger: slog.Default(), writer: log.Writer(), flags: log.Flags()}
|
||||
}
|
||||
@@ -174,7 +180,7 @@ func install(conf Config, ch *channels, opts Options) {
|
||||
old := current.Swap(st)
|
||||
slog.SetDefault(slog.New(newComponentHandler("")))
|
||||
if old != nil {
|
||||
closeChannels(old.channels)
|
||||
retireChannels(old.channels)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -186,6 +192,11 @@ func install(conf Config, ch *channels, opts Options) {
|
||||
func SetVerbosity(v int, quiet bool) {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
setVerbosityLocked(v, quiet)
|
||||
}
|
||||
|
||||
// setVerbosityLocked is SetVerbosity for a caller that holds mu.
|
||||
func setVerbosityLocked(v int, quiet bool) {
|
||||
verbosity.Store(int64(v))
|
||||
st := current.Load()
|
||||
if st == nil {
|
||||
@@ -212,18 +223,21 @@ func FromGlobals(g bonfire.Globals) Options {
|
||||
// the verbosity and quiet switch (SetVerbosity) and, when it differs from
|
||||
// the one the channels were built with, the console color mode, which
|
||||
// rebuilds the channels from the same configuration. The generated main
|
||||
// registers it with bonfire.OnGlobals.
|
||||
// registers it with bonfire.OnGlobals. It holds the setup lock for its
|
||||
// whole run, so concurrent calls rebuild the channels once.
|
||||
func ApplyGlobals(g bonfire.Globals) {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
st := current.Load()
|
||||
if st != nil && st.opts.Color != g.Color {
|
||||
opts := st.opts
|
||||
opts.Color, opts.Verbosity, opts.Quiet = g.Color, g.Verbosity, g.Quiet
|
||||
if ch, err := buildChannels(st.conf, opts); err == nil {
|
||||
install(st.conf, ch, opts)
|
||||
installLocked(st.conf, ch, opts)
|
||||
return
|
||||
}
|
||||
}
|
||||
SetVerbosity(g.Verbosity, g.Quiet)
|
||||
setVerbosityLocked(g.Verbosity, g.Quiet)
|
||||
}
|
||||
|
||||
type hideOnTTYKey struct{}
|
||||
@@ -264,6 +278,22 @@ func Close() error {
|
||||
return closeChannels(st.channels)
|
||||
}
|
||||
|
||||
// retireChannels closes the files of channels a new setup replaced, for
|
||||
// good: a goroutine that still holds the old state appends its record and
|
||||
// the file is closed again, so no descriptor stays open.
|
||||
func retireChannels(ch *channels) {
|
||||
if ch == nil {
|
||||
return
|
||||
}
|
||||
for _, c := range ch.closers {
|
||||
if r, ok := c.(interface{ retire() error }); ok {
|
||||
_ = r.retire()
|
||||
continue
|
||||
}
|
||||
_ = c.Close()
|
||||
}
|
||||
}
|
||||
|
||||
func closeChannels(ch *channels) error {
|
||||
if ch == nil {
|
||||
return nil
|
||||
@@ -275,9 +305,9 @@ func closeChannels(ch *channels) error {
|
||||
return errors.Join(errs...)
|
||||
}
|
||||
|
||||
// Reset drops the installed setup, closes its files and restores the process
|
||||
// default logger (and the standard log package's output) captured at the
|
||||
// first Bootstrap or Setup. Tests call it in t.Cleanup.
|
||||
// Reset drops the installed setup, closes its files for good and restores
|
||||
// the process default logger (and the standard log package's output)
|
||||
// captured at the first Bootstrap or Setup. Tests call it in t.Cleanup.
|
||||
func Reset() {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
@@ -290,6 +320,6 @@ func Reset() {
|
||||
saved = nil
|
||||
}
|
||||
if old != nil {
|
||||
closeChannels(old.channels)
|
||||
retireChannels(old.channels)
|
||||
}
|
||||
}
|
||||
|
||||
123
modules/diary/retire_internal_test.go
Normal file
123
modules/diary/retire_internal_test.go
Normal file
@@ -0,0 +1,123 @@
|
||||
package diary
|
||||
|
||||
import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/bonfire"
|
||||
)
|
||||
|
||||
// installDefault builds the default configuration (console and daily) on
|
||||
// root and installs it, as Setup does.
|
||||
func installDefault(t *testing.T, root string, color bonfire.ColorMode) *channels {
|
||||
t.Helper()
|
||||
opts := Options{
|
||||
Root: root,
|
||||
Stderr: io.Discard,
|
||||
Environ: []string{},
|
||||
Color: color,
|
||||
Now: func() time.Time { return time.Date(2026, 10, 21, 12, 0, 0, 0, time.Local) },
|
||||
}.normalize()
|
||||
conf := DefaultConfig()
|
||||
ch, err := buildChannels(conf, opts)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
install(conf, ch, opts)
|
||||
return ch
|
||||
}
|
||||
|
||||
// dailyOf returns the daily writer of built channels.
|
||||
func dailyOf(t *testing.T, ch *channels) *dailyWriter {
|
||||
t.Helper()
|
||||
for _, c := range ch.closers {
|
||||
if d, ok := c.(*dailyWriter); ok {
|
||||
return d
|
||||
}
|
||||
}
|
||||
t.Fatal("no daily writer in the channels")
|
||||
return nil
|
||||
}
|
||||
|
||||
func (d *dailyWriter) hasFile() bool {
|
||||
d.mu.Lock()
|
||||
defer d.mu.Unlock()
|
||||
return d.file != nil
|
||||
}
|
||||
|
||||
// TestRetiredDailyWriterKeepsNoFile (WR-06): a daily writer replaced by a
|
||||
// new setup is retired: a late record through it (from a goroutine that
|
||||
// still holds the old state) is appended to the day's file and the file is
|
||||
// closed again. diary.Close keeps letting the installed writers reopen.
|
||||
func TestRetiredDailyWriterKeepsNoFile(t *testing.T) {
|
||||
t.Cleanup(Reset)
|
||||
root := t.TempDir()
|
||||
file := filepath.Join(root, "storage", "logs", "summer-2026-10-21.log")
|
||||
|
||||
old := dailyOf(t, installDefault(t, root, bonfire.ColorNever))
|
||||
if _, err := old.Write([]byte("first\n")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
installed := dailyOf(t, installDefault(t, root, bonfire.ColorNever))
|
||||
if old.hasFile() {
|
||||
t.Fatal("the replaced writer still holds its file")
|
||||
}
|
||||
if _, err := old.Write([]byte("late\n")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if old.hasFile() {
|
||||
t.Error("a late record through the replaced writer left its file open")
|
||||
}
|
||||
|
||||
if _, err := installed.Write([]byte("current\n")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := installed.Write([]byte("reopened\n")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !installed.hasFile() {
|
||||
t.Error("the installed writer did not reopen its file after diary.Close")
|
||||
}
|
||||
raw, err := os.ReadFile(file)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := string(raw); got != "first\nlate\ncurrent\nreopened\n" {
|
||||
t.Errorf("daily file = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplyGlobalsRebuildsOnce (WR-06): concurrent ApplyGlobals calls with
|
||||
// a new color mode rebuild the channels once, and every call's verbosity
|
||||
// is applied.
|
||||
func TestApplyGlobalsRebuildsOnce(t *testing.T) {
|
||||
t.Cleanup(Reset)
|
||||
installDefault(t, t.TempDir(), bonfire.ColorAuto)
|
||||
before := gens.Load()
|
||||
var start, done sync.WaitGroup
|
||||
start.Add(1)
|
||||
for range 50 {
|
||||
done.Add(1)
|
||||
go func() {
|
||||
defer done.Done()
|
||||
start.Wait()
|
||||
ApplyGlobals(bonfire.Globals{Color: bonfire.ColorNever, Verbosity: 2})
|
||||
}()
|
||||
}
|
||||
start.Done()
|
||||
done.Wait()
|
||||
if n := gens.Load() - before; n != 1 {
|
||||
t.Errorf("the channels were rebuilt %d times, want 1", n)
|
||||
}
|
||||
st := current.Load()
|
||||
if st.opts.Color != bonfire.ColorNever || st.verbosity != 2 || Verbosity() != 2 {
|
||||
t.Errorf("state after ApplyGlobals: color %v, verbosity %d (Verbosity %d)", st.opts.Color, st.verbosity, Verbosity())
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user