Skip to content

Commit

Permalink
fix(windows-agent): Avoid leaking proservices Manager on app teardown (
Browse files Browse the repository at this point in the history
…#703)

When we call agent.Quit, we don't wait for the proServices to perform
their cleanup. This means that the process can exit while the Landscape,
registry watcher or database are in the middle of an operation.

This PR fixes this, somewhat: It guarantees to exit the process after
proservices.Stop returns. We trust that the implementation of the
teardown of these proservices is implemented properly (I gave it a quick
glance and it seems to be the case).

This problem also affected tests, as the proServices could be writing to
disk while t.Cleanup tries to delete the temp directory, causing a race
and then a test error.

---

Example test failure due to this:
https://github.com/canonical/ubuntu-pro-for-wsl/actions/runs/8436243216/job/23103334236?pr=702#step:7:63

Here, the proservices manager is still writing logs while we try to
remove the log file, even though we already called Quit on the app.
  • Loading branch information
EduardGomezEscandell authored Mar 26, 2024
2 parents ef3ee41 + 2483fdb commit 71071cd
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
2 changes: 2 additions & 0 deletions go.work.sum
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,7 @@ golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72
golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg=
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
golang.org/x/exp v0.0.0-20230905200255-921286631fa9/go.mod h1:S2oDrQGGwySpoQPVqRShND87VCbxmc6bL1Yd2oYrm6k=
golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY=
golang.org/x/mod v0.5.0/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
Expand Down Expand Up @@ -942,6 +943,7 @@ golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0=
golang.org/x/term v0.16.0/go.mod h1:yn7UURbUtPyrVJPGPq404EukNFxcm/foM+bV/bfcDsY=
golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58=
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ=
golang.org/x/text v0.5.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
Expand Down
10 changes: 6 additions & 4 deletions windows-agent/cmd/ubuntu-pro-agent/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ type App struct {
viper *viper.Viper
config daemonConfig

daemon *daemon.Daemon
daemon *daemon.Daemon
proServices *proservices.Manager

ready chan struct{}
}
Expand Down Expand Up @@ -125,7 +126,7 @@ func (a *App) serve(args ...option) error {

log.Debugf(ctx, "Agent private directory: %s", privateDir)

proservice, err := proservices.New(ctx,
proservices, err := proservices.New(ctx,
publicDir,
privateDir,
proservices.WithRegistry(opt.registry),
Expand All @@ -134,9 +135,9 @@ func (a *App) serve(args ...option) error {
close(a.ready)
return err
}
defer proservice.Stop(ctx)
a.proServices = &proservices

a.daemon = daemon.New(ctx, proservice.RegisterGRPCServices, publicDir)
a.daemon = daemon.New(ctx, proservices.RegisterGRPCServices, publicDir)

close(a.ready)

Expand Down Expand Up @@ -184,6 +185,7 @@ func (a *App) Quit() {
return
}
a.daemon.Quit(context.Background(), false)
a.proServices.Stop(context.Background())
}

// WaitReady signals when the daemon is ready
Expand Down

0 comments on commit 71071cd

Please sign in to comment.