refactor(busretry): share the D-Bus retry loop between bus services
- commit
- e61021f8684fdfdeca453b3c0a60d7252d73b8aa
- author
- brice · · 2026-10-08 08:47 UTC
- parent
- cae3aeb
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +126 -34
M Makefile +1 -1
| @@ -61,7 +61,7 @@ arch: adapter-imports | |||
| 61 | 61 | # Adapters stay independent: production code may import only the shared | |
| 62 | 62 | # adapter libraries below. outputkit holds what both output backends (drm, | |
| 63 | 63 | # headless) run; the others are leaf helpers. | |
| 64 | - | ADAPTER_LIBS := outputkit/capture|outputkit/presented|outputkit/surfaces|captureallow|clock|logging|sessionsecurity|syncfile|workspaceid|xkb | |
| 64 | + | ADAPTER_LIBS := busretry|outputkit/capture|outputkit/presented|outputkit/surfaces|captureallow|clock|logging|sessionsecurity|syncfile|workspaceid|xkb | |
| 65 | 65 | adapter-imports: | |
| 66 | 66 | @edges=$$(CGO_ENABLED=0 go list -f '{{$$p := .ImportPath}}{{range .Imports}}{{$$p}} {{.}}{{"\n"}}{{end}}' ./internal/adapters/...) || exit 1; \ | |
| 67 | 67 | bad=$$(echo "$$edges" | grep -E ' \S+/internal/adapters/' | grep -vE '/internal/adapters/($(ADAPTER_LIBS))$$' \ | |
| @@ -61,7 +61,7 @@ arch: adapter-imports | |||||
| 61 | # Adapters stay independent: production code may import only the shared | 61 | # Adapters stay independent: production code may import only the shared | ||
| 62 | # adapter libraries below. outputkit holds what both output backends (drm, | 62 | # adapter libraries below. outputkit holds what both output backends (drm, | ||
| 63 | # headless) run; the others are leaf helpers. | 63 | # headless) run; the others are leaf helpers. | ||
| 64 | - | ADAPTER_LIBS := outputkit/capture|outputkit/presented|outputkit/surfaces|captureallow|clock|logging|sessionsecurity|syncfile|workspaceid|xkb | 64 | + | ADAPTER_LIBS := busretry|outputkit/capture|outputkit/presented|outputkit/surfaces|captureallow|clock|logging|sessionsecurity|syncfile|workspaceid|xkb |
| 65 | adapter-imports: | 65 | adapter-imports: | ||
| 66 | @edges=$$(CGO_ENABLED=0 go list -f '{{$$p := .ImportPath}}{{range .Imports}}{{$$p}} {{.}}{{"\n"}}{{end}}' ./internal/adapters/...) || exit 1; \ | 66 | @edges=$$(CGO_ENABLED=0 go list -f '{{$$p := .ImportPath}}{{range .Imports}}{{$$p}} {{.}}{{"\n"}}{{end}}' ./internal/adapters/...) || exit 1; \ | ||
| 67 | bad=$$(echo "$$edges" | grep -E ' \S+/internal/adapters/' | grep -vE '/internal/adapters/($(ADAPTER_LIBS))$$' \ | 67 | bad=$$(echo "$$edges" | grep -E ' \S+/internal/adapters/' | grep -vE '/internal/adapters/($(ADAPTER_LIBS))$$' \ | ||
A internal/adapters/busretry/busretry.go +51 -0
| @@ -0,0 +1,51 @@ | |||
| 1 | + | // Package busretry keeps a D-Bus service running across bus restarts and | |
| 2 | + | // busy names: it runs an attempt again after a growing delay until the | |
| 3 | + | // context ends. | |
| 4 | + | package busretry | |
| 5 | + | ||
| 6 | + | import ( | |
| 7 | + | "context" | |
| 8 | + | "errors" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | "github.com/bnema/zerowrap" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // Max bounds the delay between attempts. | |
| 15 | + | const Max = time.Minute | |
| 16 | + | ||
| 17 | + | // Run calls attempt until ctx ends. When attempt returns, it waits retry, | |
| 18 | + | // doubling up to Max on each failure, and calls it again. An attempt that | |
| 19 | + | // ran longer than Max before failing starts the delay over: its failure is | |
| 20 | + | // new, not a retry streak. what names the feature in logs ("X ignored until | |
| 21 | + | // the next attempt"); a failure is warned once per distinct error, then | |
| 22 | + | // logged at debug level. | |
| 23 | + | func Run(ctx context.Context, retry time.Duration, what string, log zerowrap.Logger, attempt func(context.Context) error) { | |
| 24 | + | delay := retry | |
| 25 | + | var last string | |
| 26 | + | for { | |
| 27 | + | start := time.Now() | |
| 28 | + | err := attempt(ctx) | |
| 29 | + | if ctx.Err() != nil { | |
| 30 | + | return | |
| 31 | + | } | |
| 32 | + | if err == nil { | |
| 33 | + | err = errors.New(what + " stopped") | |
| 34 | + | } | |
| 35 | + | if time.Since(start) > Max { | |
| 36 | + | delay, last = retry, "" | |
| 37 | + | } | |
| 38 | + | if msg := err.Error(); msg != last { | |
| 39 | + | last = msg | |
| 40 | + | log.Warn().Err(err).Dur("retry", delay).Msg(what + " ignored until the next attempt") | |
| 41 | + | } else { | |
| 42 | + | log.Debug().Err(err).Dur("retry", delay).Msg(what + " still ignored") | |
| 43 | + | } | |
| 44 | + | select { | |
| 45 | + | case <-ctx.Done(): | |
| 46 | + | return | |
| 47 | + | case <-time.After(delay): | |
| 48 | + | } | |
| 49 | + | delay = min(2*delay, Max) | |
| 50 | + | } | |
| 51 | + | } | |
| @@ -0,0 +1,51 @@ | |||||
| 1 | + | // Package busretry keeps a D-Bus service running across bus restarts and | |||
| 2 | + | // busy names: it runs an attempt again after a growing delay until the | |||
| 3 | + | // context ends. | |||
| 4 | + | package busretry | |||
| 5 | + | ||||
| 6 | + | import ( | |||
| 7 | + | "context" | |||
| 8 | + | "errors" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | "github.com/bnema/zerowrap" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // Max bounds the delay between attempts. | |||
| 15 | + | const Max = time.Minute | |||
| 16 | + | ||||
| 17 | + | // Run calls attempt until ctx ends. When attempt returns, it waits retry, | |||
| 18 | + | // doubling up to Max on each failure, and calls it again. An attempt that | |||
| 19 | + | // ran longer than Max before failing starts the delay over: its failure is | |||
| 20 | + | // new, not a retry streak. what names the feature in logs ("X ignored until | |||
| 21 | + | // the next attempt"); a failure is warned once per distinct error, then | |||
| 22 | + | // logged at debug level. | |||
| 23 | + | func Run(ctx context.Context, retry time.Duration, what string, log zerowrap.Logger, attempt func(context.Context) error) { | |||
| 24 | + | delay := retry | |||
| 25 | + | var last string | |||
| 26 | + | for { | |||
| 27 | + | start := time.Now() | |||
| 28 | + | err := attempt(ctx) | |||
| 29 | + | if ctx.Err() != nil { | |||
| 30 | + | return | |||
| 31 | + | } | |||
| 32 | + | if err == nil { | |||
| 33 | + | err = errors.New(what + " stopped") | |||
| 34 | + | } | |||
| 35 | + | if time.Since(start) > Max { | |||
| 36 | + | delay, last = retry, "" | |||
| 37 | + | } | |||
| 38 | + | if msg := err.Error(); msg != last { | |||
| 39 | + | last = msg | |||
| 40 | + | log.Warn().Err(err).Dur("retry", delay).Msg(what + " ignored until the next attempt") | |||
| 41 | + | } else { | |||
| 42 | + | log.Debug().Err(err).Dur("retry", delay).Msg(what + " still ignored") | |||
| 43 | + | } | |||
| 44 | + | select { | |||
| 45 | + | case <-ctx.Done(): | |||
| 46 | + | return | |||
| 47 | + | case <-time.After(delay): | |||
| 48 | + | } | |||
| 49 | + | delay = min(2*delay, Max) | |||
| 50 | + | } | |||
| 51 | + | } | |||
A internal/adapters/busretry/busretry_test.go +67 -0
| @@ -0,0 +1,67 @@ | |||
| 1 | + | package busretry | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 6 | + | "testing" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/neferwl/internal/adapters/logging" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | // A failing or returning attempt runs again until ctx ends, and Run returns | |
| 13 | + | // without waiting out the delay. | |
| 14 | + | func TestRunRetriesUntilCancelled(t *testing.T) { | |
| 15 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 16 | + | calls := make(chan int, 8) | |
| 17 | + | n := 0 | |
| 18 | + | done := make(chan struct{}) | |
| 19 | + | go func() { | |
| 20 | + | defer close(done) | |
| 21 | + | Run(ctx, time.Millisecond, "test feature", logging.For(ctx, "test"), func(context.Context) error { | |
| 22 | + | n++ | |
| 23 | + | calls <- n | |
| 24 | + | if n%2 == 0 { | |
| 25 | + | return nil | |
| 26 | + | } | |
| 27 | + | return errors.New("bus gone") | |
| 28 | + | }) | |
| 29 | + | }() | |
| 30 | + | for want := 1; want <= 3; want++ { | |
| 31 | + | select { | |
| 32 | + | case got := <-calls: | |
| 33 | + | if got != want { | |
| 34 | + | t.Fatalf("attempt %d, want %d", got, want) | |
| 35 | + | } | |
| 36 | + | case <-time.After(5 * time.Second): | |
| 37 | + | t.Fatalf("attempt %d never ran", want) | |
| 38 | + | } | |
| 39 | + | } | |
| 40 | + | cancel() | |
| 41 | + | select { | |
| 42 | + | case <-done: | |
| 43 | + | case <-time.After(5 * time.Second): | |
| 44 | + | t.Fatal("Run outlived its context") | |
| 45 | + | } | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | // A long delay does not hold Run once ctx ends. | |
| 49 | + | func TestRunStopsDuringDelay(t *testing.T) { | |
| 50 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 51 | + | started := make(chan struct{}) | |
| 52 | + | done := make(chan struct{}) | |
| 53 | + | go func() { | |
| 54 | + | defer close(done) | |
| 55 | + | Run(ctx, time.Hour, "test feature", logging.For(ctx, "test"), func(context.Context) error { | |
| 56 | + | close(started) | |
| 57 | + | return errors.New("fail") | |
| 58 | + | }) | |
| 59 | + | }() | |
| 60 | + | <-started | |
| 61 | + | cancel() | |
| 62 | + | select { | |
| 63 | + | case <-done: | |
| 64 | + | case <-time.After(5 * time.Second): | |
| 65 | + | t.Fatal("Run waits out the retry delay after cancel") | |
| 66 | + | } | |
| 67 | + | } | |
| @@ -0,0 +1,67 @@ | |||||
| 1 | + | package busretry | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "errors" | |||
| 6 | + | "testing" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/neferwl/internal/adapters/logging" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | // A failing or returning attempt runs again until ctx ends, and Run returns | |||
| 13 | + | // without waiting out the delay. | |||
| 14 | + | func TestRunRetriesUntilCancelled(t *testing.T) { | |||
| 15 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 16 | + | calls := make(chan int, 8) | |||
| 17 | + | n := 0 | |||
| 18 | + | done := make(chan struct{}) | |||
| 19 | + | go func() { | |||
| 20 | + | defer close(done) | |||
| 21 | + | Run(ctx, time.Millisecond, "test feature", logging.For(ctx, "test"), func(context.Context) error { | |||
| 22 | + | n++ | |||
| 23 | + | calls <- n | |||
| 24 | + | if n%2 == 0 { | |||
| 25 | + | return nil | |||
| 26 | + | } | |||
| 27 | + | return errors.New("bus gone") | |||
| 28 | + | }) | |||
| 29 | + | }() | |||
| 30 | + | for want := 1; want <= 3; want++ { | |||
| 31 | + | select { | |||
| 32 | + | case got := <-calls: | |||
| 33 | + | if got != want { | |||
| 34 | + | t.Fatalf("attempt %d, want %d", got, want) | |||
| 35 | + | } | |||
| 36 | + | case <-time.After(5 * time.Second): | |||
| 37 | + | t.Fatalf("attempt %d never ran", want) | |||
| 38 | + | } | |||
| 39 | + | } | |||
| 40 | + | cancel() | |||
| 41 | + | select { | |||
| 42 | + | case <-done: | |||
| 43 | + | case <-time.After(5 * time.Second): | |||
| 44 | + | t.Fatal("Run outlived its context") | |||
| 45 | + | } | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | // A long delay does not hold Run once ctx ends. | |||
| 49 | + | func TestRunStopsDuringDelay(t *testing.T) { | |||
| 50 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 51 | + | started := make(chan struct{}) | |||
| 52 | + | done := make(chan struct{}) | |||
| 53 | + | go func() { | |||
| 54 | + | defer close(done) | |||
| 55 | + | Run(ctx, time.Hour, "test feature", logging.For(ctx, "test"), func(context.Context) error { | |||
| 56 | + | close(started) | |||
| 57 | + | return errors.New("fail") | |||
| 58 | + | }) | |||
| 59 | + | }() | |||
| 60 | + | <-started | |||
| 61 | + | cancel() | |||
| 62 | + | select { | |||
| 63 | + | case <-done: | |||
| 64 | + | case <-time.After(5 * time.Second): | |||
| 65 | + | t.Fatal("Run waits out the retry delay after cancel") | |||
| 66 | + | } | |||
| 67 | + | } | |||
M internal/adapters/screensaver/screensaver.go +7 -33
| @@ -12,6 +12,7 @@ import ( | |||
| 12 | 12 | "strings" | |
| 13 | 13 | "time" | |
| 14 | 14 | ||
| 15 | + | "github.com/bnema/neferwl/internal/adapters/busretry" | |
| 15 | 16 | "github.com/bnema/zerowrap" | |
| 16 | 17 | "github.com/godbus/dbus/v5" | |
| 17 | 18 | "github.com/godbus/dbus/v5/introspect" | |
| @@ -25,8 +26,6 @@ const ( | |||
| 25 | 26 | busDaemon = "org.freedesktop.DBus" | |
| 26 | 27 | // callTimeout bounds the bus calls Run makes itself. | |
| 27 | 28 | callTimeout = 3 * time.Second | |
| 28 | - | // retryMax bounds the delay between Serve's attempts. | |
| 29 | - | retryMax = time.Minute | |
| 30 | 29 | ) | |
| 31 | 30 | ||
| 32 | 31 | // ErrNameTaken is returned by Run when another program owns the name. | |
| @@ -54,40 +53,15 @@ type Reports struct { | |||
| 54 | 53 | ||
| 55 | 54 | // Serve runs the service until ctx ends. When the bus goes or another | |
| 56 | 55 | // program owns the name, it tries again after retry, doubling up to | |
| 57 | - | // retryMax, so a restarted bus or a released name is picked up. | |
| 56 | + | // busretry.Max, so a restarted bus or a released name is picked up. | |
| 58 | 57 | func Serve(ctx context.Context, address string, r Reports, retry time.Duration, log zerowrap.Logger) { | |
| 59 | - | delay := retry | |
| 60 | - | var last string | |
| 61 | - | for { | |
| 62 | - | start := time.Now() | |
| 58 | + | busretry.Run(ctx, retry, "D-Bus idle inhibitors", log, func(ctx context.Context) error { | |
| 63 | 59 | s, err := New(ctx, address, log) | |
| 64 | - | if err == nil { | |
| 65 | - | err = s.Run(ctx, r) | |
| 66 | - | } | |
| 67 | - | if ctx.Err() != nil { | |
| 68 | - | return | |
| 69 | - | } | |
| 70 | - | if err == nil { | |
| 71 | - | err = errors.New("screensaver service stopped") | |
| 60 | + | if err != nil { | |
| 61 | + | return err | |
| 72 | 62 | } | |
| 73 | - | if time.Since(start) > retryMax { | |
| 74 | - | // It served a while: the failure is new, not a retry streak. | |
| 75 | - | delay, last = retry, "" | |
| 76 | - | } | |
| 77 | - | // Warn once per kind of failure, not on every attempt. | |
| 78 | - | if msg := err.Error(); msg != last { | |
| 79 | - | last = msg | |
| 80 | - | log.Warn().Err(err).Dur("retry", delay).Msg("D-Bus idle inhibitors ignored until the next attempt") | |
| 81 | - | } else { | |
| 82 | - | log.Debug().Err(err).Dur("retry", delay).Msg("D-Bus idle inhibitors still ignored") | |
| 83 | - | } | |
| 84 | - | select { | |
| 85 | - | case <-ctx.Done(): | |
| 86 | - | return | |
| 87 | - | case <-time.After(delay): | |
| 88 | - | } | |
| 89 | - | delay = min(2*delay, retryMax) | |
| 90 | - | } | |
| 63 | + | return s.Run(ctx, r) | |
| 64 | + | }) | |
| 91 | 65 | } | |
| 92 | 66 | ||
| 93 | 67 | // Service owns one private session bus connection. Run must be called once. | |
| @@ -12,6 +12,7 @@ import ( | |||||
| 12 | "strings" | 12 | "strings" | ||
| 13 | "time" | 13 | "time" | ||
| 14 | 14 | ||||
| 15 | + | "github.com/bnema/neferwl/internal/adapters/busretry" | |||
| 15 | "github.com/bnema/zerowrap" | 16 | "github.com/bnema/zerowrap" | ||
| 16 | "github.com/godbus/dbus/v5" | 17 | "github.com/godbus/dbus/v5" | ||
| 17 | "github.com/godbus/dbus/v5/introspect" | 18 | "github.com/godbus/dbus/v5/introspect" | ||
| @@ -25,8 +26,6 @@ const ( | |||||
| 25 | busDaemon = "org.freedesktop.DBus" | 26 | busDaemon = "org.freedesktop.DBus" | ||
| 26 | // callTimeout bounds the bus calls Run makes itself. | 27 | // callTimeout bounds the bus calls Run makes itself. | ||
| 27 | callTimeout = 3 * time.Second | 28 | callTimeout = 3 * time.Second | ||
| 28 | - | // retryMax bounds the delay between Serve's attempts. | |||
| 29 | - | retryMax = time.Minute | |||
| 30 | ) | 29 | ) | ||
| 31 | 30 | ||||
| 32 | // ErrNameTaken is returned by Run when another program owns the name. | 31 | // ErrNameTaken is returned by Run when another program owns the name. | ||
| @@ -54,40 +53,15 @@ type Reports struct { | |||||
| 54 | 53 | ||||
| 55 | // Serve runs the service until ctx ends. When the bus goes or another | 54 | // Serve runs the service until ctx ends. When the bus goes or another | ||
| 56 | // program owns the name, it tries again after retry, doubling up to | 55 | // program owns the name, it tries again after retry, doubling up to | ||
| 57 | - | // retryMax, so a restarted bus or a released name is picked up. | 56 | + | // busretry.Max, so a restarted bus or a released name is picked up. |
| 58 | func Serve(ctx context.Context, address string, r Reports, retry time.Duration, log zerowrap.Logger) { | 57 | func Serve(ctx context.Context, address string, r Reports, retry time.Duration, log zerowrap.Logger) { | ||
| 59 | - | delay := retry | 58 | + | busretry.Run(ctx, retry, "D-Bus idle inhibitors", log, func(ctx context.Context) error { |
| 60 | - | var last string | |||
| 61 | - | for { | |||
| 62 | - | start := time.Now() | |||
| 63 | s, err := New(ctx, address, log) | 59 | s, err := New(ctx, address, log) | ||
| 64 | - | if err == nil { | 60 | + | if err != nil { |
| 65 | - | err = s.Run(ctx, r) | 61 | + | return err |
| 66 | - | } | |||
| 67 | - | if ctx.Err() != nil { | |||
| 68 | - | return | |||
| 69 | - | } | |||
| 70 | - | if err == nil { | |||
| 71 | - | err = errors.New("screensaver service stopped") | |||
| 72 | } | 62 | } | ||
| 73 | - | if time.Since(start) > retryMax { | 63 | + | return s.Run(ctx, r) |
| 74 | - | // It served a while: the failure is new, not a retry streak. | 64 | + | }) |
| 75 | - | delay, last = retry, "" | |||
| 76 | - | } | |||
| 77 | - | // Warn once per kind of failure, not on every attempt. | |||
| 78 | - | if msg := err.Error(); msg != last { | |||
| 79 | - | last = msg | |||
| 80 | - | log.Warn().Err(err).Dur("retry", delay).Msg("D-Bus idle inhibitors ignored until the next attempt") | |||
| 81 | - | } else { | |||
| 82 | - | log.Debug().Err(err).Dur("retry", delay).Msg("D-Bus idle inhibitors still ignored") | |||
| 83 | - | } | |||
| 84 | - | select { | |||
| 85 | - | case <-ctx.Done(): | |||
| 86 | - | return | |||
| 87 | - | case <-time.After(delay): | |||
| 88 | - | } | |||
| 89 | - | delay = min(2*delay, retryMax) | |||
| 90 | - | } | |||
| 91 | } | 65 | } | ||
| 92 | 66 | ||||
| 93 | // Service owns one private session bus connection. Run must be called once. | 67 | // Service owns one private session bus connection. Run must be called once. | ||