From 63230d3a3cffaf025ed5998305cb0dcb3a9d95e8 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 21 Oct 2024 11:22:31 -0700 Subject: [PATCH] darwin notifier listen after show desktop (#1900) --- cmd/launcher/desktop.go | 13 ++++++------- ee/desktop/user/notify/notify_darwin.go | 12 +++++++----- ee/desktop/user/notify/notify_linux.go | 5 ++++- ee/desktop/user/notify/notify_windows.go | 5 ++++- go.mod | 2 +- go.sum | 4 ++-- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/cmd/launcher/desktop.go b/cmd/launcher/desktop.go index e9fbe291a..b4a647de9 100644 --- a/cmd/launcher/desktop.go +++ b/cmd/launcher/desktop.go @@ -131,6 +131,7 @@ func runDesktop(_ *multislogger.MultiSlogger, args []string) error { // Set up notification sending and listening notifier := notify.NewDesktopNotifier(slogger, *flIconPath) + runGroup.Add("desktopNotifier", notifier.Execute, notifier.Interrupt) server, err := userserver.New(slogger, *flUserServerAuthToken, *flUserServerSocketPath, shutdownChan, showDesktopChan, notifier) if err != nil { @@ -183,13 +184,11 @@ func runDesktop(_ *multislogger.MultiSlogger, args []string) error { } }() - go func() { - // wait to show desktop until we get the signal from root - <-showDesktopChan - runGroup.Add("desktopNotifier", notifier.Listen, notifier.Interrupt) - systray.Show() - }() - + <-showDesktopChan + // on darwin, if notifier.Listen() is called on a blocked main thread, it causes a crash, + // so we wait until the main thread is unblocked to call it before initializing the menu. + // this is noop for non-darwin + notifier.Listen() // blocks until shutdown called m.Init() return nil diff --git a/ee/desktop/user/notify/notify_darwin.go b/ee/desktop/user/notify/notify_darwin.go index ee64bbd0b..4673bce16 100644 --- a/ee/desktop/user/notify/notify_darwin.go +++ b/ee/desktop/user/notify/notify_darwin.go @@ -33,11 +33,7 @@ func NewDesktopNotifier(_ *slog.Logger, _ string) *macNotifier { } } -func (m *macNotifier) Listen() error { - if isBundle() { - C.runNotificationListenerApp() - } - +func (m *macNotifier) Execute() error { <-m.interrupt return nil } @@ -46,6 +42,12 @@ func (m *macNotifier) Interrupt(err error) { m.interrupt <- struct{}{} } +func (m *macNotifier) Listen() { + if isBundle() { + C.runNotificationListenerApp() + } +} + func (m *macNotifier) SendNotification(n Notification) error { // Check if we're running inside a bundle -- if we aren't, we should not attempt to send // a notification because it will cause a panic. diff --git a/ee/desktop/user/notify/notify_linux.go b/ee/desktop/user/notify/notify_linux.go index faf4d8ff8..c4cd8deaf 100644 --- a/ee/desktop/user/notify/notify_linux.go +++ b/ee/desktop/user/notify/notify_linux.go @@ -55,7 +55,7 @@ func NewDesktopNotifier(slogger *slog.Logger, iconFilepath string) *dbusNotifier } } -func (d *dbusNotifier) Listen() error { +func (d *dbusNotifier) Execute() error { if d.conn != nil { if err := d.conn.AddMatchSignal( dbus.WithMatchObjectPath(notificationServiceObj), @@ -138,6 +138,9 @@ func (d *dbusNotifier) Interrupt(err error) { ) } +// just make compiler happy, this is only needed on darwin +func (d *dbusNotifier) Listen() {} + func (d *dbusNotifier) SendNotification(n Notification) error { if err := d.sendNotificationViaDbus(n); err == nil { return nil diff --git a/ee/desktop/user/notify/notify_windows.go b/ee/desktop/user/notify/notify_windows.go index fe2828f11..021affefe 100644 --- a/ee/desktop/user/notify/notify_windows.go +++ b/ee/desktop/user/notify/notify_windows.go @@ -23,11 +23,14 @@ func NewDesktopNotifier(_ *slog.Logger, iconFilepath string) *windowsNotifier { // Listen doesn't do anything on Windows -- the `launch` variable in the notification XML // automatically handles opening URLs for us. -func (w *windowsNotifier) Listen() error { +func (w *windowsNotifier) Execute() error { <-w.interrupt return nil } +// just make compiler happy, this is only needed on darwin +func (w *windowsNotifier) Listen() {} + func (w *windowsNotifier) Interrupt(err error) { w.interrupt <- struct{}{} } diff --git a/go.mod b/go.mod index ebe135c34..9277c19a5 100644 --- a/go.mod +++ b/go.mod @@ -51,7 +51,7 @@ require ( github.com/golang-migrate/migrate/v4 v4.16.2 github.com/golang/snappy v0.0.4 github.com/kolide/goleveldb v0.0.0-20240514204455-8d30cd4d31c6 - github.com/kolide/systray v1.10.5-0.20241018174140-3d1b0664c945 + github.com/kolide/systray v1.10.5-0.20241021175748-13aef6380bdb github.com/kolide/toast v1.0.2 github.com/saltosystems/winrt-go v0.0.0-20240510082706-db61b37f5877 github.com/shirou/gopsutil/v3 v3.23.3 diff --git a/go.sum b/go.sum index 561cb47e7..f3c0c6afa 100644 --- a/go.sum +++ b/go.sum @@ -178,8 +178,8 @@ github.com/kolide/kit v0.0.0-20240411131714-94dd1939cf50 h1:N7RaYBPTK5o4y2z1z8kl github.com/kolide/kit v0.0.0-20240411131714-94dd1939cf50/go.mod h1:pFbEKXFww1uqu4RRO7qCnUmQ2EIwKYRzUqpJbODNlfc= github.com/kolide/krypto v0.1.1-0.20231229162826-db516b7e0121 h1:f7APX9VNsCkD/tdlAjbU4A22FyfTOCF6QadlvnzZElg= github.com/kolide/krypto v0.1.1-0.20231229162826-db516b7e0121/go.mod h1:/0sxd3OIxciTlMTeZI/9WTaUHsx/K/+3f+NbD5dywTY= -github.com/kolide/systray v1.10.5-0.20241018174140-3d1b0664c945 h1:NO+UQe89tISQj+rS0l+hrB4BAF3oKdLGhZLddC5weXM= -github.com/kolide/systray v1.10.5-0.20241018174140-3d1b0664c945/go.mod h1:rnPcECLGM9DY/+fb3O9WNKiPBPMCdNg3rkyadLWOCTU= +github.com/kolide/systray v1.10.5-0.20241021175748-13aef6380bdb h1:d2pfEh5Yd6+C+D096YQlHwcq28TPOjoeoG+lCQojkJM= +github.com/kolide/systray v1.10.5-0.20241021175748-13aef6380bdb/go.mod h1:rnPcECLGM9DY/+fb3O9WNKiPBPMCdNg3rkyadLWOCTU= github.com/kolide/toast v1.0.2 h1:BQlIfO3wbKIEWfF0c8v4UkdhSIZYnSWaKkZl+Yarptk= github.com/kolide/toast v1.0.2/go.mod h1:OguLiOUf57YSEuZqjfk4uP4KdT0QOblGoySOI8F1I0Y= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=