Skip to content

Stop sleep monitor on user requested disconnect and restart on connect #4922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hurricanehrndz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Stop sleep monitor on user requested disconnect and restart on connect #4922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions client/ui/client_ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@

client.setupSignalHandler(client.ctx)

client.setDefaultFonts()

Check failure on line 126 in client/ui/client_ui.go

View workflow job for this annotation

GitHub Actions / JS / Lint 

client.setDefaultFonts undefined (type *serviceClient has no field or method setDefaultFonts) (typecheck)
systray.Run(client.onTrayReady, client.onTrayExit)
}

Expand Down Expand Up @@ -320,6 +320,9 @@
logFile string
wLoginURL fyne.Window

sleepService *sleep.Service
sleepLock sync.Mutex

connectCancel context.CancelFunc
}

Expand Down Expand Up @@ -1165,6 +1168,14 @@

// startSleepListener initializes the sleep detection service and listens for sleep events
func (s *serviceClient) startSleepListener() {
s.sleepLock.Lock()
defer s.sleepLock.Unlock()

if s.sleepService != nil {
log.Debug("sleep detection service already initialized")
return
}
Comment on lines +1174 to +1177
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The early return check for existing sleep service occurs after acquiring the lock but doesn't prevent a potential race condition. If startSleepListener() is called multiple times concurrently, both calls could pass the nil check before either assigns to s.sleepService. Consider checking if s.sleepService != nil again after the sleep service is successfully created but before assignment, or document that this method should only be called from the event handler which is already serialized.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hurricanehrndz hurricanehrndz Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Contributor Author

@hurricanehrndz hurricanehrndz Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


sleepService, err := sleep.New()
if err != nil {
log.Warnf("%v", err)
Expand All @@ -1176,18 +1187,32 @@
return
}

s.sleepService = sleepService
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the sleep service creation succeeds but Start() fails, s.sleepService will be assigned but never started. This leaves the service in an inconsistent state. Consider only assigning s.sleepService after both creation and Start() succeed, and ensure proper cleanup if Start() fails.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hurricanehrndz hurricanehrndz Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

log.Info("sleep detection service initialized")

// Cleanup on context cancellation
go func() {
<-s.ctx.Done()
log.Info("stopping sleep event listener")
if err := sleepService.Deregister(); err != nil {
log.Errorf("failed to deregister sleep detection: %v", err)
}
s.stopSleepListener()
}()
}

// stopSleepListener stops the sleep detection service
func (s *serviceClient) stopSleepListener() {
s.sleepLock.Lock()
defer s.sleepLock.Unlock()

if s.sleepService == nil {
return
}

log.Info("stopping sleep event listener")
if err := s.sleepService.Deregister(); err != nil {
log.Errorf("failed to deregister sleep detection: %v", err)
}
s.sleepService = nil
}

// handleSleepEvents sends a sleep notification to the daemon via gRPC
func (s *serviceClient) handleSleepEvents(event sleep.EventType) {
conn, err := s.getSrvClient(0)
Expand Down
3 changes: 3 additions & 0 deletions client/ui/event_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@

go func() {
defer connectCancel()
defer h.client.startSleepListener()

if err := h.client.menuUpClick(connectCtx); err != nil {
st, ok := status.FromError(err)
Expand All @@ -105,6 +106,8 @@
h.client.connectCancel = nil
}

h.client.stopSleepListener()

go func() {
if err := h.client.menuDownClick(); err != nil {
st, ok := status.FromError(err)
Expand Down Expand Up @@ -208,7 +211,7 @@
}

func (h *eventHandler) handleUpdateClick() {
if err := openURL(version.DownloadUrl()); err != nil {

Check failure on line 214 in client/ui/event_handler.go

View workflow job for this annotation

GitHub Actions / JS / Lint 

undefined: version.DownloadUrl (typecheck)
log.Errorf("failed to open download URL: %v", err)
}
}
Expand Down
Loading